[PATCH 5.4] smb3: fix problem with null cifs super block with previous patch

[PATCH 5.4] smb3: fix problem with null cifs super block with previous patch

[Pratyush Yadav]

From: Steve French <stfrench@microsoft.com>

[ Upstream commit 87f93d82e0952da18af4d978e7d887b4c5326c0b ]

Add check for null cifs_sb to create_options helper

Signed-off-by: Steve French <stfrench@microsoft.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
---

Only compile-tested. This was discovered by our static code analysis
tool. I do not use CIFS and do not know how to actually reproduce the
NULL dereference.

Follow up from [0]. Original patch is at [1].

Mandatory text due to licensing terms:

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

[0] https://lore.kernel.org/stable/20230405114220.108739-1-ptyadav@amazon.de/T/#u
[1] https://lore.kernel.org/all/CAH2r5mtu69KEWU94qZK32H_8cvyhVU8GyOKrZqbdjH0ZLd95Zg@mail.gmail.com/

 fs/cifs/cifsproto.h | 2 +-
 fs/cifs/smb2ops.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index a5fab9afd699f..2dde83a969680 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -602,7 +602,7 @@ static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,

 static inline int cifs_create_options(struct cifs_sb_info *cifs_sb, int options)
 {
-	if (backup_cred(cifs_sb))
+	if (cifs_sb && (backup_cred(cifs_sb)))
 		return options | CREATE_OPEN_BACKUP_INTENT;
 	else
 		return options;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 4cb0ebe7330eb..44a261b9850de 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2343,7 +2343,7 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
 				      FS_FULL_SIZE_INFORMATION,
 				      SMB2_O_INFO_FILESYSTEM,
 				      sizeof(struct smb2_fs_full_size_info),
-				      &rsp_iov, &buftype, NULL);
+				      &rsp_iov, &buftype, cifs_sb);
 	if (rc)
 		goto qfs_exit;

--
2.39.2

Re: [PATCH 5.4] smb3: fix problem with null cifs super block with previous patch

[Greg Kroah-Hartman]

On Wed, Apr 05, 2023 at 03:57:09PM +0200, Pratyush Yadav wrote:
> From: Steve French <stfrench@microsoft.com>
> 
> [ Upstream commit 87f93d82e0952da18af4d978e7d887b4c5326c0b ]
> 
> Add check for null cifs_sb to create_options helper
> 
> Signed-off-by: Steve French <stfrench@microsoft.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
> ---
> 
> Only compile-tested. This was discovered by our static code analysis
> tool. I do not use CIFS and do not know how to actually reproduce the
> NULL dereference.
> 
> Follow up from [0]. Original patch is at [1].
> 
> Mandatory text due to licensing terms:
> 
> This bug was discovered and resolved using Coverity Static Analysis
> Security Testing (SAST) by Synopsys, Inc.

What?  That's funny.  And nothing I'm going to be adding to the
changelog text, sorry, as that's not what is upstream.  Please go poke
your lawyers, that's not ok.

thanks,

greg k-h

Re: [PATCH 5.4] smb3: fix problem with null cifs super block with previous patch

[Pratyush Yadav]

On Wed, Apr 05 2023, Greg Kroah-Hartman wrote:

> On Wed, Apr 05, 2023 at 03:57:09PM +0200, Pratyush Yadav wrote:
>> From: Steve French <stfrench@microsoft.com>
>>
>> [ Upstream commit 87f93d82e0952da18af4d978e7d887b4c5326c0b ]
>>
>> Add check for null cifs_sb to create_options helper
>>
>> Signed-off-by: Steve French <stfrench@microsoft.com>
>> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
>> ---
>>
>> Only compile-tested. This was discovered by our static code analysis
>> tool. I do not use CIFS and do not know how to actually reproduce the
>> NULL dereference.
>>
>> Follow up from [0]. Original patch is at [1].
>>
>> Mandatory text due to licensing terms:
>>
>> This bug was discovered and resolved using Coverity Static Analysis
>> Security Testing (SAST) by Synopsys, Inc.
>
> What?  That's funny.  And nothing I'm going to be adding to the
> changelog text, sorry, as that's not what is upstream.

That is fine by me. I placed this text below the 3 dashed lines so it
does _not_ end up in the commit message, but still discloses this
information.

> Please go poke your lawyers, that's not ok.

Yes, perhaps I should. But let's go forward with this patch since it
keeps the original commit message?

-- 
Regards,
Pratyush Yadav



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH 5.4] smb3: fix problem with null cifs super block with previous patch

[Greg Kroah-Hartman]

On Wed, Apr 05, 2023 at 04:34:00PM +0200, Pratyush Yadav wrote:
> On Wed, Apr 05 2023, Greg Kroah-Hartman wrote:
> 
> > On Wed, Apr 05, 2023 at 03:57:09PM +0200, Pratyush Yadav wrote:
> >> From: Steve French <stfrench@microsoft.com>
> >>
> >> [ Upstream commit 87f93d82e0952da18af4d978e7d887b4c5326c0b ]
> >>
> >> Add check for null cifs_sb to create_options helper
> >>
> >> Signed-off-by: Steve French <stfrench@microsoft.com>
> >> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
> >> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
> >> ---
> >>
> >> Only compile-tested. This was discovered by our static code analysis
> >> tool. I do not use CIFS and do not know how to actually reproduce the
> >> NULL dereference.
> >>
> >> Follow up from [0]. Original patch is at [1].
> >>
> >> Mandatory text due to licensing terms:
> >>
> >> This bug was discovered and resolved using Coverity Static Analysis
> >> Security Testing (SAST) by Synopsys, Inc.
> >
> > What?  That's funny.  And nothing I'm going to be adding to the
> > changelog text, sorry, as that's not what is upstream.
> 
> That is fine by me. I placed this text below the 3 dashed lines so it
> does _not_ end up in the commit message, but still discloses this
> information.
> 
> > Please go poke your lawyers, that's not ok.
> 
> Yes, perhaps I should. But let's go forward with this patch since it
> keeps the original commit message?

It's already been queued up, you should have gotten an email saying
that, right?

thanks,

greg k-h

Re: [PATCH 5.4] smb3: fix problem with null cifs super block with previous patch

[Pratyush Yadav]

On Wed, Apr 05 2023, Greg Kroah-Hartman wrote:

> On Wed, Apr 05, 2023 at 04:34:00PM +0200, Pratyush Yadav wrote:
>> On Wed, Apr 05 2023, Greg Kroah-Hartman wrote:
>>
>> > On Wed, Apr 05, 2023 at 03:57:09PM +0200, Pratyush Yadav wrote:
>> >> From: Steve French <stfrench@microsoft.com>
>> >>
>> >> [ Upstream commit 87f93d82e0952da18af4d978e7d887b4c5326c0b ]
>> >>
>> >> Add check for null cifs_sb to create_options helper
>> >>
>> >> Signed-off-by: Steve French <stfrench@microsoft.com>
>> >> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>> >> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>> >> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
>> >> ---
>> >>
>> >> Only compile-tested. This was discovered by our static code analysis
>> >> tool. I do not use CIFS and do not know how to actually reproduce the
>> >> NULL dereference.
>> >>
>> >> Follow up from [0]. Original patch is at [1].
>> >>
>> >> Mandatory text due to licensing terms:
>> >>
>> >> This bug was discovered and resolved using Coverity Static Analysis
>> >> Security Testing (SAST) by Synopsys, Inc.
>> >
>> > What?  That's funny.  And nothing I'm going to be adding to the
>> > changelog text, sorry, as that's not what is upstream.
>>
>> That is fine by me. I placed this text below the 3 dashed lines so it
>> does _not_ end up in the commit message, but still discloses this
>> information.
>>
>> > Please go poke your lawyers, that's not ok.
>>
>> Yes, perhaps I should. But let's go forward with this patch since it
>> keeps the original commit message?
>
> It's already been queued up, you should have gotten an email saying
> that, right?

Yes, I did, thanks! There was a bit of a race in me sending that email
and receiving the notification.

-- 
Regards,
Pratyush Yadav



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879