[PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[Ghadi Elie Rahme]

[ Upstream commit 6916881f443f67f6893b504fa2171468c8aed915 ]

It is possible for find_ipc_from_server_path to run while the tcon is NULL,
resulting in a NULL pointer dereference crash when calling strcasecmp().
This happens when the ipc connection fails freeing the tcon and setting it
to NULL while the dfs cache worker thread was already executing.
This issue was fixed upstream indirectly by a rewrite that removed this
function. Although with this fix the issue can still occur, the window of
the race is now much narrower.
A fix that would completely fix it using mutexes was tested and
worked fine. However the regression potential would be much higher and so
would be the deviation from upstream.
This is a good balance of safety while minimizing upstream deviation.

Stack trace:

 BUG: kernel NULL pointer dereference, address: 0000000000000050
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 1013dc64067 P4D 10125f65067 PUD 10125f64067 PMD 0
 Oops: 0000 [#1] SMP NOPTI
 CPU: 80 PID: 3913754 Comm: kworker/u256:1 Kdump: loaded Not tainted 5.15.0-143-generic #153-Ubuntu
 Hardware name: Dell Inc. PowerEdge R760/09XV41, BIOS 2.3.5 09/10/2024
 Workqueue: cifs-dfscache refresh_cache_worker [cifs]
 RIP: 0010:strcasecmp+0x19/0x50
 Code: 01 84 c9 75 f1 c3 cc cc cc cc 0f 1f 80 00 00 00 00 49 89 f9 31 ff 41 0f b6 04 39 0f b6 c8 89 c2 83 c2 20 f6 81 e0 39 89 85  01 <0f> b6 0c 3e 0f b6 d2 0f 45 c2 89 ca 44 0f b6 c1 83 c2 20 41 f6 80
 RSP: 0018:ff4043e68aadb900 EFLAGS: 00010246
 RAX: 000000000000005c RBX: ff4043e68aadbc68 RCX: 000000000000005c
 RDX: 000000000000007c RSI: 0000000000000050 RDI: 0000000000000000
 RBP: ff4043e68aadb990 R08: 0000000000000064 R09: ff4043e68aadb91f
 R10: 0000000000000012 R11: 0000000000000000 R12: ff210c171f193c00
 R13: 0000000000000009 R14: 0000000000000008 R15: ff210d1d3f19a7c0
 FS:  0000000000000000(0000) GS:ff210d127fc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000050 CR3: 0000011f2cc38005 CR4: 0000000000771ee0
 PKRU: 55555554
 Call Trace:
 <TASK>
 ? find_ipc_from_server_path+0xd9/0x110 [cifs]
 refresh_cache+0xf1/0x470 [cifs]
 ? in4_pton+0x7a/0x160
 ? kfree+0x1f7/0x250
 ? target_share_equal+0x198/0x210 [cifs]
 ? __refresh_tcon.isra.0+0x242/0x670 [cifs]
 ? kfree+0x1f7/0x250
 ? __refresh_tcon.isra.0+0x242/0x670 [cifs]
 ? cifs_put_tcon.part.0+0x39/0x220 [cifs]
 ? cifs_put_tcon+0x1c/0x30 [cifs]
 ? refresh_mounts+0x147/0x210 [cifs]
 refresh_cache_worker+0x1ac/0x300 [cifs]
 ? lock_timer_base+0x3b/0xd0
 process_one_work+0x228/0x3d0
 worker_thread+0x53/0x420
 ? process_one_work+0x3d0/0x3d0
 kthread+0x127/0x150
 ? set_kthread_struct+0x50/0x50
 ret_from_fork+0x1f/0x30
 </TASK>

Fixes: c870a8e70e68 ("cifs: handle different charsets in dfs cache")
Cc: Steve French <sfrench@samba.org>
Cc: Paulo Alcantara <pc@cjr.nz>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Aurelien Aptel <aaptel@suse.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Signed-off-by: Ghadi Elie Rahme <ghadi.rahme@canonical.com>
---
Changes in v2:
- Added maintainers to CC.

v2: https://lore.kernel.org/stable/20260303173139.517020-1-ghadi.rahme@canonical.com/T/#u

 fs/smb/client/dfs_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
index 3bc1d3494be3..af2177b96f2f 100644
--- a/fs/smb/client/dfs_cache.c
+++ b/fs/smb/client/dfs_cache.c
@@ -98,7 +98,7 @@ static struct cifs_ses *find_ipc_from_server_path(struct cifs_ses **ses, const c

 	get_ipc_unc(path, unc, sizeof(unc));
 	for (; *ses; ses++) {
-		if (!strcasecmp(unc, (*ses)->tcon_ipc->tree_name))
+		if ((*ses)->tcon_ipc && !strcasecmp(unc, (*ses)->tcon_ipc->tree_name))
 			return *ses;
 	}
 	return ERR_PTR(-ENOENT);
--
2.51.0

Re: [PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[Greg KH]

On Thu, Mar 19, 2026 at 04:49:29PM +0200, Ghadi Elie Rahme wrote:
> [ Upstream commit 6916881f443f67f6893b504fa2171468c8aed915 ]

Again, wrong git id :(

Re: [PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[Ghadi Rahme]

 > Again, wrong git id :(

Thank you for reviewing this.

There is not direct fix to this issue upstream however upstream is no 
longer affected by this issue.

The commit ID I referenced is the commit that indirectly resolved this 
issue by completely refactoring the code which led to the removal of the 
function I patched.

Is there a better way I can convey this in a V3 maybe?

Re: [PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[Greg KH]

On Tue, Mar 31, 2026 at 09:59:01AM +0300, Ghadi Rahme wrote:
> > Again, wrong git id :(
> 
> Thank you for reviewing this.
> 
> There is not direct fix to this issue upstream however upstream is no longer
> affected by this issue.

Then why not backport the specific changes that caused newer kernels to
not be affected?

> The commit ID I referenced is the commit that indirectly resolved this issue
> by completely refactoring the code which led to the removal of the function
> I patched.
> 
> Is there a better way I can convey this in a V3 maybe?

Backport the same changes?

thanks,

greg k-h

Re: [PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[Ghadi Rahme]

 > Then why not backport the specific changes that caused newer kernels 
to not be affected?

Based on the documentation [1] for submitting patches to stable, the 
patch cannot be over 100 lines long with context.

The upstream patch exceeds this limit by a lot and cherry picking the 
specific changes from it that remove this function is not feasible 
without causing the driver to break. In other words the removal of 
"find_ipc_from_server_path" is dependent on this refactor.

 > Backport the same changes?

I can go ahead with this solution, given I get the green light to ignore 
the 100 line rule.

[1] https://www.kernel.org/doc/html/v4.11/process/stable-kernel-rules.html#

Regards,

Ghadi

Re: [PATCH v2 6.1.y] smb/dfs_cache: Fix NULL pointer dereference on session connection failure

[Greg KH]

On Tue, Mar 31, 2026 at 10:54:08AM +0300, Ghadi Rahme wrote:
> > Then why not backport the specific changes that caused newer kernels to
> not be affected?
> 
> Based on the documentation [1] for submitting patches to stable, the patch
> cannot be over 100 lines long with context.
> 
> The upstream patch exceeds this limit by a lot and cherry picking the
> specific changes from it that remove this function is not feasible without
> causing the driver to break. In other words the removal of
> "find_ipc_from_server_path" is dependent on this refactor.
> 
> > Backport the same changes?
> 
> I can go ahead with this solution, given I get the green light to ignore the
> 100 line rule.
> 
> [1] https://www.kernel.org/doc/html/v4.11/process/stable-kernel-rules.html#

We take "large" backports from mainline all the time, as long as the
maintainers for the subsystem are ok with it.  to take one-off patches
is usually much harder as the change itself is almost always "wrong",
and then future changes are even harder to backport.

So I recommend working with the maintainers here please and see what
they want to do, if anything.

thanks,

greg k-h