* [PATCH] cifs: Fix null-ptr-deref by static initializing global lock
@ 2025-08-06 13:22 Yunseong Kim
2025-08-06 20:09 ` Steve French
0 siblings, 1 reply; 3+ messages in thread
From: Yunseong Kim @ 2025-08-06 13:22 UTC (permalink / raw)
To: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM
Cc: Namjae Jeon, linux-cifs, samba-technical, linux-kernel,
Yunseong Kim
A kernel panic can be triggered by reading /proc/fs/cifs/debug_dirs.
The crash is a null-ptr-deref inside spin_lock(), caused by the use of the
uninitialized global spinlock cifs_tcp_ses_lock.
init_cifs()
└── cifs_proc_init()
└── // User can access /proc/fs/cifs/debug_dirs here
└── cifs_debug_dirs_proc_show()
└── spin_lock(&cifs_tcp_ses_lock); // Uninitialized!
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
Mem abort info:
ESR = 0x0000000096000005
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x05: level 1 translation fault
Data abort info:
ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[dfff800000000000] address between user and kernel address ranges
Internal error: Oops: 0000000096000005 [#1] SMP
Modules linked in:
CPU: 3 UID: 0 PID: 16435 Comm: stress-ng-procf Not tainted 6.16.0-10385-g79f14b5d84c6 #37 PREEMPT
Hardware name: QEMU KVM Virtual Machine, BIOS 2025.02-8ubuntu1 06/11/2025
pstate: 23400005 (nzCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
pc : do_raw_spin_lock+0x84/0x2cc
lr : _raw_spin_lock+0x24/0x34
sp : ffff8000966477e0
x29: ffff800096647860 x28: ffff800096647b88 x27: ffff0001c0c22070
x26: ffff0003eb2b60c8 x25: ffff0001c0c22018 x24: dfff800000000000
x23: ffff0000f624e000 x22: ffff0003eb2b6020 x21: ffff0000f624e768
x20: 0000000000000004 x19: 0000000000000000 x18: 0000000000000000
x17: 0000000000000000 x16: ffff8000804b9600 x15: ffff700012cc8f04
x14: 1ffff00012cc8f04 x13: 0000000000000004 x12: ffffffffffffffff
x11: 1ffff00012cc8f00 x10: ffff80008d9af0d2 x9 : f3f3f304f1f1f1f1
x8 : 0000000000000000 x7 : 7365733c203e6469 x6 : 20656572743c2023
x5 : ffff0000e0ce0044 x4 : ffff80008a4deb6e x3 : ffff8000804b9718
x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
do_raw_spin_lock+0x84/0x2cc (P)
_raw_spin_lock+0x24/0x34
cifs_debug_dirs_proc_show+0x1ac/0x4c0
seq_read_iter+0x3b0/0xc28
proc_reg_read_iter+0x178/0x2a8
vfs_read+0x5f8/0x88c
ksys_read+0x120/0x210
__arm64_sys_read+0x7c/0x90
invoke_syscall+0x98/0x2b8
el0_svc_common+0x130/0x23c
do_el0_svc+0x48/0x58
el0_svc+0x40/0x140
el0t_64_sync_handler+0x84/0x12c
el0t_64_sync+0x1ac/0x1b0
Code: aa0003f3 f9000feb f2fe7e69 f8386969 (38f86908)
---[ end trace 0000000000000000 ]---
The root cause is an initialization order problem. The lock is declared
as a global variable and intended to be initialized during module startup.
However, the procfs entry that uses this lock can be accessed by userspace
before the spin_lock_init() call has run. This creates a race window where
reading the proc file will attempt to use the lock before it is
initialized, leading to the crash.
For a global lock with a static lifetime, the correct and robust approach
is to use compile-time initialization.
Fixes: 844e5c0eb176 ("smb3 client: add way to show directory leases for improved debugging")
Signed-off-by: Yunseong Kim <ysk@kzalloc.com>
---
fs/smb/client/cifsfs.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 31930b7266db..3bd85ab2deb1 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -77,7 +77,7 @@ unsigned int global_secflags = CIFSSEC_DEF;
unsigned int GlobalCurrentXid; /* protected by GlobalMid_Lock */
unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Lock */
unsigned int GlobalMaxActiveXid; /* prot by GlobalMid_Lock */
-spinlock_t GlobalMid_Lock; /* protects above & list operations on midQ entries */
+DEFINE_SPINLOCK(GlobalMid_Lock); /* protects above & list operations on midQ entries */
/*
* Global counters, updated atomically
@@ -97,7 +97,7 @@ atomic_t total_buf_alloc_count;
atomic_t total_small_buf_alloc_count;
#endif/* STATS2 */
struct list_head cifs_tcp_ses_list;
-spinlock_t cifs_tcp_ses_lock;
+DEFINE_SPINLOCK(cifs_tcp_ses_lock);
static const struct super_operations cifs_super_ops;
unsigned int CIFSMaxBufSize = CIFS_MAX_MSGSIZE;
module_param(CIFSMaxBufSize, uint, 0444);
@@ -1863,8 +1863,6 @@ init_cifs(void)
GlobalCurrentXid = 0;
GlobalTotalActiveXid = 0;
GlobalMaxActiveXid = 0;
- spin_lock_init(&cifs_tcp_ses_lock);
- spin_lock_init(&GlobalMid_Lock);
cifs_lock_secret = get_random_u32();
--
2.50.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cifs: Fix null-ptr-deref by static initializing global lock
2025-08-06 13:22 [PATCH] cifs: Fix null-ptr-deref by static initializing global lock Yunseong Kim
@ 2025-08-06 20:09 ` Steve French
2025-08-07 6:21 ` Shyam Prasad N
0 siblings, 1 reply; 3+ messages in thread
From: Steve French @ 2025-08-06 20:09 UTC (permalink / raw)
To: Yunseong Kim
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, Namjae Jeon, linux-cifs, samba-technical,
linux-kernel
merged into cifs-2.6.git for-next
On Wed, Aug 6, 2025 at 8:23 AM Yunseong Kim <ysk@kzalloc.com> wrote:
>
> A kernel panic can be triggered by reading /proc/fs/cifs/debug_dirs.
> The crash is a null-ptr-deref inside spin_lock(), caused by the use of the
> uninitialized global spinlock cifs_tcp_ses_lock.
>
> init_cifs()
> └── cifs_proc_init()
> └── // User can access /proc/fs/cifs/debug_dirs here
> └── cifs_debug_dirs_proc_show()
> └── spin_lock(&cifs_tcp_ses_lock); // Uninitialized!
>
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> Mem abort info:
> ESR = 0x0000000096000005
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x05: level 1 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [dfff800000000000] address between user and kernel address ranges
> Internal error: Oops: 0000000096000005 [#1] SMP
> Modules linked in:
> CPU: 3 UID: 0 PID: 16435 Comm: stress-ng-procf Not tainted 6.16.0-10385-g79f14b5d84c6 #37 PREEMPT
> Hardware name: QEMU KVM Virtual Machine, BIOS 2025.02-8ubuntu1 06/11/2025
> pstate: 23400005 (nzCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> pc : do_raw_spin_lock+0x84/0x2cc
> lr : _raw_spin_lock+0x24/0x34
> sp : ffff8000966477e0
> x29: ffff800096647860 x28: ffff800096647b88 x27: ffff0001c0c22070
> x26: ffff0003eb2b60c8 x25: ffff0001c0c22018 x24: dfff800000000000
> x23: ffff0000f624e000 x22: ffff0003eb2b6020 x21: ffff0000f624e768
> x20: 0000000000000004 x19: 0000000000000000 x18: 0000000000000000
> x17: 0000000000000000 x16: ffff8000804b9600 x15: ffff700012cc8f04
> x14: 1ffff00012cc8f04 x13: 0000000000000004 x12: ffffffffffffffff
> x11: 1ffff00012cc8f00 x10: ffff80008d9af0d2 x9 : f3f3f304f1f1f1f1
> x8 : 0000000000000000 x7 : 7365733c203e6469 x6 : 20656572743c2023
> x5 : ffff0000e0ce0044 x4 : ffff80008a4deb6e x3 : ffff8000804b9718
> x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> do_raw_spin_lock+0x84/0x2cc (P)
> _raw_spin_lock+0x24/0x34
> cifs_debug_dirs_proc_show+0x1ac/0x4c0
> seq_read_iter+0x3b0/0xc28
> proc_reg_read_iter+0x178/0x2a8
> vfs_read+0x5f8/0x88c
> ksys_read+0x120/0x210
> __arm64_sys_read+0x7c/0x90
> invoke_syscall+0x98/0x2b8
> el0_svc_common+0x130/0x23c
> do_el0_svc+0x48/0x58
> el0_svc+0x40/0x140
> el0t_64_sync_handler+0x84/0x12c
> el0t_64_sync+0x1ac/0x1b0
> Code: aa0003f3 f9000feb f2fe7e69 f8386969 (38f86908)
> ---[ end trace 0000000000000000 ]---
>
> The root cause is an initialization order problem. The lock is declared
> as a global variable and intended to be initialized during module startup.
> However, the procfs entry that uses this lock can be accessed by userspace
> before the spin_lock_init() call has run. This creates a race window where
> reading the proc file will attempt to use the lock before it is
> initialized, leading to the crash.
>
> For a global lock with a static lifetime, the correct and robust approach
> is to use compile-time initialization.
>
> Fixes: 844e5c0eb176 ("smb3 client: add way to show directory leases for improved debugging")
> Signed-off-by: Yunseong Kim <ysk@kzalloc.com>
> ---
> fs/smb/client/cifsfs.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> index 31930b7266db..3bd85ab2deb1 100644
> --- a/fs/smb/client/cifsfs.c
> +++ b/fs/smb/client/cifsfs.c
> @@ -77,7 +77,7 @@ unsigned int global_secflags = CIFSSEC_DEF;
> unsigned int GlobalCurrentXid; /* protected by GlobalMid_Lock */
> unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Lock */
> unsigned int GlobalMaxActiveXid; /* prot by GlobalMid_Lock */
> -spinlock_t GlobalMid_Lock; /* protects above & list operations on midQ entries */
> +DEFINE_SPINLOCK(GlobalMid_Lock); /* protects above & list operations on midQ entries */
>
> /*
> * Global counters, updated atomically
> @@ -97,7 +97,7 @@ atomic_t total_buf_alloc_count;
> atomic_t total_small_buf_alloc_count;
> #endif/* STATS2 */
> struct list_head cifs_tcp_ses_list;
> -spinlock_t cifs_tcp_ses_lock;
> +DEFINE_SPINLOCK(cifs_tcp_ses_lock);
> static const struct super_operations cifs_super_ops;
> unsigned int CIFSMaxBufSize = CIFS_MAX_MSGSIZE;
> module_param(CIFSMaxBufSize, uint, 0444);
> @@ -1863,8 +1863,6 @@ init_cifs(void)
> GlobalCurrentXid = 0;
> GlobalTotalActiveXid = 0;
> GlobalMaxActiveXid = 0;
> - spin_lock_init(&cifs_tcp_ses_lock);
> - spin_lock_init(&GlobalMid_Lock);
>
> cifs_lock_secret = get_random_u32();
>
> --
> 2.50.0
>
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cifs: Fix null-ptr-deref by static initializing global lock
2025-08-06 20:09 ` Steve French
@ 2025-08-07 6:21 ` Shyam Prasad N
0 siblings, 0 replies; 3+ messages in thread
From: Shyam Prasad N @ 2025-08-07 6:21 UTC (permalink / raw)
To: Steve French
Cc: Yunseong Kim, Steve French, Paulo Alcantara, Ronnie Sahlberg,
Shyam Prasad N, Tom Talpey, Bharath SM, Namjae Jeon, linux-cifs,
samba-technical, linux-kernel
On Thu, Aug 7, 2025 at 1:44 AM Steve French <smfrench@gmail.com> wrote:
>
> merged into cifs-2.6.git for-next
>
> On Wed, Aug 6, 2025 at 8:23 AM Yunseong Kim <ysk@kzalloc.com> wrote:
> >
> > A kernel panic can be triggered by reading /proc/fs/cifs/debug_dirs.
> > The crash is a null-ptr-deref inside spin_lock(), caused by the use of the
> > uninitialized global spinlock cifs_tcp_ses_lock.
> >
> > init_cifs()
> > └── cifs_proc_init()
> > └── // User can access /proc/fs/cifs/debug_dirs here
> > └── cifs_debug_dirs_proc_show()
> > └── spin_lock(&cifs_tcp_ses_lock); // Uninitialized!
> >
> > KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> > Mem abort info:
> > ESR = 0x0000000096000005
> > EC = 0x25: DABT (current EL), IL = 32 bits
> > SET = 0, FnV = 0
> > EA = 0, S1PTW = 0
> > FSC = 0x05: level 1 translation fault
> > Data abort info:
> > ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
> > CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> > [dfff800000000000] address between user and kernel address ranges
> > Internal error: Oops: 0000000096000005 [#1] SMP
> > Modules linked in:
> > CPU: 3 UID: 0 PID: 16435 Comm: stress-ng-procf Not tainted 6.16.0-10385-g79f14b5d84c6 #37 PREEMPT
> > Hardware name: QEMU KVM Virtual Machine, BIOS 2025.02-8ubuntu1 06/11/2025
> > pstate: 23400005 (nzCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> > pc : do_raw_spin_lock+0x84/0x2cc
> > lr : _raw_spin_lock+0x24/0x34
> > sp : ffff8000966477e0
> > x29: ffff800096647860 x28: ffff800096647b88 x27: ffff0001c0c22070
> > x26: ffff0003eb2b60c8 x25: ffff0001c0c22018 x24: dfff800000000000
> > x23: ffff0000f624e000 x22: ffff0003eb2b6020 x21: ffff0000f624e768
> > x20: 0000000000000004 x19: 0000000000000000 x18: 0000000000000000
> > x17: 0000000000000000 x16: ffff8000804b9600 x15: ffff700012cc8f04
> > x14: 1ffff00012cc8f04 x13: 0000000000000004 x12: ffffffffffffffff
> > x11: 1ffff00012cc8f00 x10: ffff80008d9af0d2 x9 : f3f3f304f1f1f1f1
> > x8 : 0000000000000000 x7 : 7365733c203e6469 x6 : 20656572743c2023
> > x5 : ffff0000e0ce0044 x4 : ffff80008a4deb6e x3 : ffff8000804b9718
> > x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000000
> > Call trace:
> > do_raw_spin_lock+0x84/0x2cc (P)
> > _raw_spin_lock+0x24/0x34
> > cifs_debug_dirs_proc_show+0x1ac/0x4c0
> > seq_read_iter+0x3b0/0xc28
> > proc_reg_read_iter+0x178/0x2a8
> > vfs_read+0x5f8/0x88c
> > ksys_read+0x120/0x210
> > __arm64_sys_read+0x7c/0x90
> > invoke_syscall+0x98/0x2b8
> > el0_svc_common+0x130/0x23c
> > do_el0_svc+0x48/0x58
> > el0_svc+0x40/0x140
> > el0t_64_sync_handler+0x84/0x12c
> > el0t_64_sync+0x1ac/0x1b0
> > Code: aa0003f3 f9000feb f2fe7e69 f8386969 (38f86908)
> > ---[ end trace 0000000000000000 ]---
> >
> > The root cause is an initialization order problem. The lock is declared
> > as a global variable and intended to be initialized during module startup.
> > However, the procfs entry that uses this lock can be accessed by userspace
> > before the spin_lock_init() call has run. This creates a race window where
> > reading the proc file will attempt to use the lock before it is
> > initialized, leading to the crash.
> >
> > For a global lock with a static lifetime, the correct and robust approach
> > is to use compile-time initialization.
> >
> > Fixes: 844e5c0eb176 ("smb3 client: add way to show directory leases for improved debugging")
> > Signed-off-by: Yunseong Kim <ysk@kzalloc.com>
> > ---
> > fs/smb/client/cifsfs.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> > index 31930b7266db..3bd85ab2deb1 100644
> > --- a/fs/smb/client/cifsfs.c
> > +++ b/fs/smb/client/cifsfs.c
> > @@ -77,7 +77,7 @@ unsigned int global_secflags = CIFSSEC_DEF;
> > unsigned int GlobalCurrentXid; /* protected by GlobalMid_Lock */
> > unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Lock */
> > unsigned int GlobalMaxActiveXid; /* prot by GlobalMid_Lock */
> > -spinlock_t GlobalMid_Lock; /* protects above & list operations on midQ entries */
> > +DEFINE_SPINLOCK(GlobalMid_Lock); /* protects above & list operations on midQ entries */
> >
> > /*
> > * Global counters, updated atomically
> > @@ -97,7 +97,7 @@ atomic_t total_buf_alloc_count;
> > atomic_t total_small_buf_alloc_count;
> > #endif/* STATS2 */
> > struct list_head cifs_tcp_ses_list;
> > -spinlock_t cifs_tcp_ses_lock;
> > +DEFINE_SPINLOCK(cifs_tcp_ses_lock);
> > static const struct super_operations cifs_super_ops;
> > unsigned int CIFSMaxBufSize = CIFS_MAX_MSGSIZE;
> > module_param(CIFSMaxBufSize, uint, 0444);
> > @@ -1863,8 +1863,6 @@ init_cifs(void)
> > GlobalCurrentXid = 0;
> > GlobalTotalActiveXid = 0;
> > GlobalMaxActiveXid = 0;
> > - spin_lock_init(&cifs_tcp_ses_lock);
> > - spin_lock_init(&GlobalMid_Lock);
> >
> > cifs_lock_secret = get_random_u32();
> >
> > --
> > 2.50.0
> >
> >
>
>
> --
> Thanks,
>
> Steve
>
Good catch.
But the problem is that cifs_proc_init gets called very early on in init_cifs.
That call should be moved to just before (or possibly after)
registering the filesystem.
--
Regards,
Shyam
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-07 6:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 13:22 [PATCH] cifs: Fix null-ptr-deref by static initializing global lock Yunseong Kim
2025-08-06 20:09 ` Steve French
2025-08-07 6:21 ` Shyam Prasad N
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).