[RFC,2/2] CIFS: Fix NULL ptr deref

Message ID 20180604202935.4872-3-aaptel@suse.com
State New
Headers show
Series
  • fix for-next kernel crashes
Related show

Commit Message

Aurélien Aptel June 4, 2018, 8:29 p.m.
cifs->master_tlink is NULL against Win Server 2016 (which is
strange.. not sure why) and is dereferenced in cifs_sb_master_tcon().

move master_tlink getter to cifsglob.h so it can be used from
smb2misc.c

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/cifsglob.h | 6 ++++++
 fs/cifs/connect.c  | 6 ------
 fs/cifs/smb2misc.c | 4 +++-
 3 files changed, 9 insertions(+), 7 deletions(-)

Comments

Ronnie Sahlberg June 4, 2018, 11:10 p.m. | #1
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>


Try to find out why master_tlink is NULL here, it should be, right?



----- Original Message -----
> From: "Aurelien Aptel" <aaptel@suse.com>
> To: linux-cifs@vger.kernel.org
> Cc: lsahlber@redhat.com, smfrench@gmail.com, "Aurelien Aptel" <aaptel@suse.com>
> Sent: Tuesday, 5 June, 2018 6:29:35 AM
> Subject: [PATCH RFC 2/2] CIFS: Fix NULL ptr deref
> 
> cifs->master_tlink is NULL against Win Server 2016 (which is
> strange.. not sure why) and is dereferenced in cifs_sb_master_tcon().
> 
> move master_tlink getter to cifsglob.h so it can be used from
> smb2misc.c
> 
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/cifsglob.h | 6 ++++++
>  fs/cifs/connect.c  | 6 ------
>  fs/cifs/smb2misc.c | 4 +++-
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 08d1cdd96701..1efa2e65bc1a 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1019,6 +1019,12 @@ tlink_tcon(struct tcon_link *tlink)
>  	return tlink->tl_tcon;
>  }
>  
> +static inline struct tcon_link *
> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
> +{
> +	return cifs_sb->master_tlink;
> +}
> +
>  extern void cifs_put_tlink(struct tcon_link *tlink);
>  
>  static inline struct tcon_link *
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index e5a2fe7f0dd4..9089b73809de 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3116,12 +3116,6 @@ cifs_put_tlink(struct tcon_link *tlink)
>  	return;
>  }
>  
> -static inline struct tcon_link *
> -cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
> -{
> -	return cifs_sb->master_tlink;
> -}
> -
>  static int
>  compare_mount_options(struct super_block *sb, struct cifs_mnt_data
>  *mnt_data)
>  {
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index cb5728e3d87d..e2bec47c6845 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -453,8 +453,10 @@ cifs_convert_path_to_utf16(const char *from, struct
> cifs_sb_info *cifs_sb)
>  		start_of_path = from + 1;
>  #ifdef CONFIG_CIFS_SMB311
>  	/* SMB311 POSIX extensions paths do not include leading slash */
> -	else if (cifs_sb_master_tcon(cifs_sb)->posix_extensions)
> +	else if (cifs_sb_master_tlink(cifs_sb) &&
> +		 cifs_sb_master_tcon(cifs_sb)->posix_extensions) {
>  		start_of_path = from + 1;
> +	}
>  #endif /* 311 */
>  	else
>  		start_of_path = from;
> --
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French June 5, 2018, 12:15 a.m. | #2
I tried it to Windows 10 a few minutes ago and it didn't fail - maybe
DFS enabled on Win2016?!

On Mon, Jun 4, 2018 at 6:10 PM, Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
>
>
> Try to find out why master_tlink is NULL here, it should be, right?
>
>
>
> ----- Original Message -----
>> From: "Aurelien Aptel" <aaptel@suse.com>
>> To: linux-cifs@vger.kernel.org
>> Cc: lsahlber@redhat.com, smfrench@gmail.com, "Aurelien Aptel" <aaptel@suse.com>
>> Sent: Tuesday, 5 June, 2018 6:29:35 AM
>> Subject: [PATCH RFC 2/2] CIFS: Fix NULL ptr deref
>>
>> cifs->master_tlink is NULL against Win Server 2016 (which is
>> strange.. not sure why) and is dereferenced in cifs_sb_master_tcon().
>>
>> move master_tlink getter to cifsglob.h so it can be used from
>> smb2misc.c
>>
>> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
>> ---
>>  fs/cifs/cifsglob.h | 6 ++++++
>>  fs/cifs/connect.c  | 6 ------
>>  fs/cifs/smb2misc.c | 4 +++-
>>  3 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 08d1cdd96701..1efa2e65bc1a 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -1019,6 +1019,12 @@ tlink_tcon(struct tcon_link *tlink)
>>       return tlink->tl_tcon;
>>  }
>>
>> +static inline struct tcon_link *
>> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
>> +{
>> +     return cifs_sb->master_tlink;
>> +}
>> +
>>  extern void cifs_put_tlink(struct tcon_link *tlink);
>>
>>  static inline struct tcon_link *
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index e5a2fe7f0dd4..9089b73809de 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -3116,12 +3116,6 @@ cifs_put_tlink(struct tcon_link *tlink)
>>       return;
>>  }
>>
>> -static inline struct tcon_link *
>> -cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
>> -{
>> -     return cifs_sb->master_tlink;
>> -}
>> -
>>  static int
>>  compare_mount_options(struct super_block *sb, struct cifs_mnt_data
>>  *mnt_data)
>>  {
>> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
>> index cb5728e3d87d..e2bec47c6845 100644
>> --- a/fs/cifs/smb2misc.c
>> +++ b/fs/cifs/smb2misc.c
>> @@ -453,8 +453,10 @@ cifs_convert_path_to_utf16(const char *from, struct
>> cifs_sb_info *cifs_sb)
>>               start_of_path = from + 1;
>>  #ifdef CONFIG_CIFS_SMB311
>>       /* SMB311 POSIX extensions paths do not include leading slash */
>> -     else if (cifs_sb_master_tcon(cifs_sb)->posix_extensions)
>> +     else if (cifs_sb_master_tlink(cifs_sb) &&
>> +              cifs_sb_master_tcon(cifs_sb)->posix_extensions) {
>>               start_of_path = from + 1;
>> +     }
>>  #endif /* 311 */
>>       else
>>               start_of_path = from;
>> --
>> 2.13.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
Steve French June 7, 2018, 12:12 p.m. | #3
merged into cifs-2.6.git

On Mon, Jun 4, 2018 at 3:29 PM, Aurelien Aptel <aaptel@suse.com> wrote:
> cifs->master_tlink is NULL against Win Server 2016 (which is
> strange.. not sure why) and is dereferenced in cifs_sb_master_tcon().
>
> move master_tlink getter to cifsglob.h so it can be used from
> smb2misc.c
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/cifsglob.h | 6 ++++++
>  fs/cifs/connect.c  | 6 ------
>  fs/cifs/smb2misc.c | 4 +++-
>  3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 08d1cdd96701..1efa2e65bc1a 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1019,6 +1019,12 @@ tlink_tcon(struct tcon_link *tlink)
>         return tlink->tl_tcon;
>  }
>
> +static inline struct tcon_link *
> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
> +{
> +       return cifs_sb->master_tlink;
> +}
> +
>  extern void cifs_put_tlink(struct tcon_link *tlink);
>
>  static inline struct tcon_link *
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index e5a2fe7f0dd4..9089b73809de 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3116,12 +3116,6 @@ cifs_put_tlink(struct tcon_link *tlink)
>         return;
>  }
>
> -static inline struct tcon_link *
> -cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
> -{
> -       return cifs_sb->master_tlink;
> -}
> -
>  static int
>  compare_mount_options(struct super_block *sb, struct cifs_mnt_data *mnt_data)
>  {
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index cb5728e3d87d..e2bec47c6845 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -453,8 +453,10 @@ cifs_convert_path_to_utf16(const char *from, struct cifs_sb_info *cifs_sb)
>                 start_of_path = from + 1;
>  #ifdef CONFIG_CIFS_SMB311
>         /* SMB311 POSIX extensions paths do not include leading slash */
> -       else if (cifs_sb_master_tcon(cifs_sb)->posix_extensions)
> +       else if (cifs_sb_master_tlink(cifs_sb) &&
> +                cifs_sb_master_tcon(cifs_sb)->posix_extensions) {
>                 start_of_path = from + 1;
> +       }
>  #endif /* 311 */
>         else
>                 start_of_path = from;
> --
> 2.13.6
>

Patch

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 08d1cdd96701..1efa2e65bc1a 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1019,6 +1019,12 @@  tlink_tcon(struct tcon_link *tlink)
 	return tlink->tl_tcon;
 }
 
+static inline struct tcon_link *
+cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
+{
+	return cifs_sb->master_tlink;
+}
+
 extern void cifs_put_tlink(struct tcon_link *tlink);
 
 static inline struct tcon_link *
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index e5a2fe7f0dd4..9089b73809de 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3116,12 +3116,6 @@  cifs_put_tlink(struct tcon_link *tlink)
 	return;
 }
 
-static inline struct tcon_link *
-cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
-{
-	return cifs_sb->master_tlink;
-}
-
 static int
 compare_mount_options(struct super_block *sb, struct cifs_mnt_data *mnt_data)
 {
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index cb5728e3d87d..e2bec47c6845 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -453,8 +453,10 @@  cifs_convert_path_to_utf16(const char *from, struct cifs_sb_info *cifs_sb)
 		start_of_path = from + 1;
 #ifdef CONFIG_CIFS_SMB311
 	/* SMB311 POSIX extensions paths do not include leading slash */
-	else if (cifs_sb_master_tcon(cifs_sb)->posix_extensions)
+	else if (cifs_sb_master_tlink(cifs_sb) &&
+		 cifs_sb_master_tcon(cifs_sb)->posix_extensions) {
 		start_of_path = from + 1;
+	}
 #endif /* 311 */
 	else
 		start_of_path = from;