linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cifs: missing null pointer check in cifs_mount
@ 2021-06-23  1:17 Steve French
  2021-06-23 11:48 ` Aurélien Aptel
  0 siblings, 1 reply; 4+ messages in thread
From: Steve French @ 2021-06-23  1:17 UTC (permalink / raw)
  To: CIFS; +Cc: Paulo Alcantara, ronnie sahlberg

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

We weren't checking if tcon is null before setting dfs path,
although we check for null tcon in an earlier assignment statement.

Addresses-Coverity: 1476411 ("Dereference after null check")
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/connect.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8d95607a9312..196ef9ec69db 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3480,7 +3480,8 @@ int cifs_mount(struct cifs_sb_info *cifs_sb,
struct smb3_fs_context *ctx)
  goto error;
  }
  spin_lock(&cifs_tcp_ses_lock);
- tcon->dfs_path = ref_path;
+ if (tcon)
+ tcon->dfs_path = ref_path;
  ref_path = NULL;
  spin_unlock(&cifs_tcp_ses_lock);


-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-missing-null-pointer-check-in-cifs_mount.patch --]
[-- Type: text/x-patch, Size: 968 bytes --]

From 632096b66b2fa2621e3d2d02c2c2fd436975810b Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Tue, 22 Jun 2021 20:13:44 -0500
Subject: [PATCH] cifs: missing null pointer check in cifs_mount

We weren't checking if tcon is null before setting dfs path,
although we check for null tcon in an earlier assignment statement.

Addresses-Coverity: 1476411 ("Dereference after null check")
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/connect.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8d95607a9312..196ef9ec69db 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3480,7 +3480,8 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
 		goto error;
 	}
 	spin_lock(&cifs_tcp_ses_lock);
-	tcon->dfs_path = ref_path;
+	if (tcon)
+		tcon->dfs_path = ref_path;
 	ref_path = NULL;
 	spin_unlock(&cifs_tcp_ses_lock);
 
-- 
2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: missing null pointer check in cifs_mount
  2021-06-23  1:17 [PATCH] cifs: missing null pointer check in cifs_mount Steve French
@ 2021-06-23 11:48 ` Aurélien Aptel
  2021-06-23 12:17   ` Paulo Alcantara
  0 siblings, 1 reply; 4+ messages in thread
From: Aurélien Aptel @ 2021-06-23 11:48 UTC (permalink / raw)
  To: Steve French, CIFS; +Cc: Paulo Alcantara, ronnie sahlberg

Steve French <smfrench@gmail.com> writes:
> We weren't checking if tcon is null before setting dfs path,
> although we check for null tcon in an earlier assignment statement.

If tcon is NULL there is no point in continuing in that function, we
should have exited earlier.

If tcon is NULL it means mount_get_conns() failed so presumably rc will
be != 0 and we would goto error.

I don't think this is needed. We could change the existing check after
the loop to this you really want to be safe:

	if (rc || !tcon)
		goto error;


Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: missing null pointer check in cifs_mount
  2021-06-23 11:48 ` Aurélien Aptel
@ 2021-06-23 12:17   ` Paulo Alcantara
  2021-06-24  0:34     ` Steve French
  0 siblings, 1 reply; 4+ messages in thread
From: Paulo Alcantara @ 2021-06-23 12:17 UTC (permalink / raw)
  To: Aurélien Aptel, Steve French, CIFS; +Cc: ronnie sahlberg

Agreed.

On June 23, 2021 8:48:24 AM GMT-03:00, "Aurélien Aptel" <aaptel@suse.com> wrote:
>Steve French <smfrench@gmail.com> writes:
>> We weren't checking if tcon is null before setting dfs path,
>> although we check for null tcon in an earlier assignment statement.
>
>If tcon is NULL there is no point in continuing in that function, we
>should have exited earlier.
>
>If tcon is NULL it means mount_get_conns() failed so presumably rc will
>be != 0 and we would goto error.
>
>I don't think this is needed. We could change the existing check after
>the loop to this you really want to be safe:
>
>	if (rc || !tcon)
>		goto error;
>
>
>Cheers,

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: missing null pointer check in cifs_mount
  2021-06-23 12:17   ` Paulo Alcantara
@ 2021-06-24  0:34     ` Steve French
  0 siblings, 0 replies; 4+ messages in thread
From: Steve French @ 2021-06-24  0:34 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: Aurélien Aptel, CIFS, ronnie sahlberg

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

updated patch attached with Aurelien's suggestion.

On Wed, Jun 23, 2021 at 7:17 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> Agreed.
>
> On June 23, 2021 8:48:24 AM GMT-03:00, "Aurélien Aptel" <aaptel@suse.com> wrote:
> >Steve French <smfrench@gmail.com> writes:
> >> We weren't checking if tcon is null before setting dfs path,
> >> although we check for null tcon in an earlier assignment statement.
> >
> >If tcon is NULL there is no point in continuing in that function, we
> >should have exited earlier.
> >
> >If tcon is NULL it means mount_get_conns() failed so presumably rc will
> >be != 0 and we would goto error.
> >
> >I don't think this is needed. We could change the existing check after
> >the loop to this you really want to be safe:
> >
> >       if (rc || !tcon)
> >               goto error;
> >
> >
> >Cheers,



-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-missing-null-pointer-check-in-cifs_mount.patch --]
[-- Type: text/x-patch, Size: 899 bytes --]

From 162004a2f7ef5c77600e364dc4e9315b0e6ca386 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 23 Jun 2021 19:32:24 -0500
Subject: [PATCH] cifs: missing null pointer check in cifs_mount

We weren't checking if tcon is null before setting dfs path,
although we check for null tcon in an earlier assignment statement.

Addresses-Coverity: 1476411 ("Dereference after null check")
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8d95607a9312..c8079376d294 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3451,7 +3451,7 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
 			rc = -ELOOP;
 	} while (rc == -EREMOTE);
 
-	if (rc)
+	if (rc || !tcon)
 		goto error;
 
 	kfree(ref_path);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-24  0:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23  1:17 [PATCH] cifs: missing null pointer check in cifs_mount Steve French
2021-06-23 11:48 ` Aurélien Aptel
2021-06-23 12:17   ` Paulo Alcantara
2021-06-24  0:34     ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).