diff mbox series

smb: client: fix oops due to unset link speed

Message ID 20250116172903.666243-1-pc@manguebit.com (mailing list archive)
State New
Headers show
Series smb: client: fix oops due to unset link speed | expand

Commit Message

Paulo Alcantara Jan. 16, 2025, 5:29 p.m. UTC
It isn't guaranteed that NETWORK_INTERFACE_INFO::LinkSpeed will always
be set by the server, so the client must handle any values and then
prevent oopses like below from happening:

Oops: divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI
CPU: 0 UID: 0 PID: 1323 Comm: cat Not tainted 6.13.0-rc7 #2
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-3.fc41
04/01/2014
RIP: 0010:cifs_debug_data_proc_show+0xa45/0x1460 [cifs] Code: 00 00 48
89 df e8 3b cd 1b c1 41 f6 44 24 2c 04 0f 84 50 01 00 00 48 89 ef e8
e7 d0 1b c1 49 8b 44 24 18 31 d2 49 8d 7c 24 28 <48> f7 74 24 18 48 89
c3 e8 6e cf 1b c1 41 8b 6c 24 28 49 8d 7c 24
RSP: 0018:ffffc90001817be0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff88811230022c RCX: ffffffffc041bd99
RDX: 0000000000000000 RSI: 0000000000000567 RDI: ffff888112300228
RBP: ffff888112300218 R08: fffff52000302f5f R09: ffffed1022fa58ac
R10: ffff888117d2c566 R11: 00000000fffffffe R12: ffff888112300200
R13: 000000012a15343f R14: 0000000000000001 R15: ffff888113f2db58
FS: 00007fe27119e740(0000) GS:ffff888148600000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe2633c5000 CR3: 0000000124da0000 CR4: 0000000000750ef0
PKRU: 55555554
Call Trace:
 <TASK>
 ? __die_body.cold+0x19/0x27
 ? die+0x2e/0x50
 ? do_trap+0x159/0x1b0
 ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
 ? do_error_trap+0x90/0x130
 ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
 ? exc_divide_error+0x39/0x50
 ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
 ? asm_exc_divide_error+0x1a/0x20
 ? cifs_debug_data_proc_show+0xa39/0x1460 [cifs]
 ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
 ? seq_read_iter+0x42e/0x790
 seq_read_iter+0x19a/0x790
 proc_reg_read_iter+0xbe/0x110
 ? __pfx_proc_reg_read_iter+0x10/0x10
 vfs_read+0x469/0x570
 ? do_user_addr_fault+0x398/0x760
 ? __pfx_vfs_read+0x10/0x10
 ? find_held_lock+0x8a/0xa0
 ? __pfx_lock_release+0x10/0x10
 ksys_read+0xd3/0x170
 ? __pfx_ksys_read+0x10/0x10
 ? __rcu_read_unlock+0x50/0x270
 ? mark_held_locks+0x1a/0x90
 do_syscall_64+0xbb/0x1d0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe271288911
Code: 00 48 8b 15 01 25 10 00 f7 d8 64 89 02 b8 ff ff ff ff eb bd e8
20 ad 01 00 f3 0f 1e fa 80 3d b5 a7 10 00 00 74 13 31 c0 0f 05 <48> 3d
00 f0 ff ff 77 4f c3 66 0f 1f 44 00 00 55 48 89 e5 48 83 ec
RSP: 002b:00007ffe87c079d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000040000 RCX: 00007fe271288911
RDX: 0000000000040000 RSI: 00007fe2633c6000 RDI: 0000000000000003
RBP: 00007ffe87c07a00 R08: 0000000000000000 R09: 00007fe2713e6380
R10: 0000000000000022 R11: 0000000000000246 R12: 0000000000040000
R13: 00007fe2633c6000 R14: 0000000000000003 R15: 0000000000000000
 </TASK>

Fix this by setting cifs_server_iface::speed to a sane value (1Gbps)
by default when link speed is unset.

Cc: Shyam Prasad N <nspmangalore@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Fixes: a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed")
Reported-by: Frank Sorenson <sorenson@redhat.com>
Reported-by: Jay Shin <jaeshin@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
---
 fs/smb/client/smb2ops.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Steve French Jan. 16, 2025, 5:32 p.m. UTC | #1
merged into cifs-2.6.git for-next pending additional review and testing

On Thu, Jan 16, 2025 at 11:29 AM Paulo Alcantara <pc@manguebit.com> wrote:
>
> It isn't guaranteed that NETWORK_INTERFACE_INFO::LinkSpeed will always
> be set by the server, so the client must handle any values and then
> prevent oopses like below from happening:
>
> Oops: divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI
> CPU: 0 UID: 0 PID: 1323 Comm: cat Not tainted 6.13.0-rc7 #2
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-3.fc41
> 04/01/2014
> RIP: 0010:cifs_debug_data_proc_show+0xa45/0x1460 [cifs] Code: 00 00 48
> 89 df e8 3b cd 1b c1 41 f6 44 24 2c 04 0f 84 50 01 00 00 48 89 ef e8
> e7 d0 1b c1 49 8b 44 24 18 31 d2 49 8d 7c 24 28 <48> f7 74 24 18 48 89
> c3 e8 6e cf 1b c1 41 8b 6c 24 28 49 8d 7c 24
> RSP: 0018:ffffc90001817be0 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff88811230022c RCX: ffffffffc041bd99
> RDX: 0000000000000000 RSI: 0000000000000567 RDI: ffff888112300228
> RBP: ffff888112300218 R08: fffff52000302f5f R09: ffffed1022fa58ac
> R10: ffff888117d2c566 R11: 00000000fffffffe R12: ffff888112300200
> R13: 000000012a15343f R14: 0000000000000001 R15: ffff888113f2db58
> FS: 00007fe27119e740(0000) GS:ffff888148600000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fe2633c5000 CR3: 0000000124da0000 CR4: 0000000000750ef0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  ? __die_body.cold+0x19/0x27
>  ? die+0x2e/0x50
>  ? do_trap+0x159/0x1b0
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? do_error_trap+0x90/0x130
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? exc_divide_error+0x39/0x50
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? asm_exc_divide_error+0x1a/0x20
>  ? cifs_debug_data_proc_show+0xa39/0x1460 [cifs]
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? seq_read_iter+0x42e/0x790
>  seq_read_iter+0x19a/0x790
>  proc_reg_read_iter+0xbe/0x110
>  ? __pfx_proc_reg_read_iter+0x10/0x10
>  vfs_read+0x469/0x570
>  ? do_user_addr_fault+0x398/0x760
>  ? __pfx_vfs_read+0x10/0x10
>  ? find_held_lock+0x8a/0xa0
>  ? __pfx_lock_release+0x10/0x10
>  ksys_read+0xd3/0x170
>  ? __pfx_ksys_read+0x10/0x10
>  ? __rcu_read_unlock+0x50/0x270
>  ? mark_held_locks+0x1a/0x90
>  do_syscall_64+0xbb/0x1d0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fe271288911
> Code: 00 48 8b 15 01 25 10 00 f7 d8 64 89 02 b8 ff ff ff ff eb bd e8
> 20 ad 01 00 f3 0f 1e fa 80 3d b5 a7 10 00 00 74 13 31 c0 0f 05 <48> 3d
> 00 f0 ff ff 77 4f c3 66 0f 1f 44 00 00 55 48 89 e5 48 83 ec
> RSP: 002b:00007ffe87c079d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 0000000000040000 RCX: 00007fe271288911
> RDX: 0000000000040000 RSI: 00007fe2633c6000 RDI: 0000000000000003
> RBP: 00007ffe87c07a00 R08: 0000000000000000 R09: 00007fe2713e6380
> R10: 0000000000000022 R11: 0000000000000246 R12: 0000000000040000
> R13: 00007fe2633c6000 R14: 0000000000000003 R15: 0000000000000000
>  </TASK>
>
> Fix this by setting cifs_server_iface::speed to a sane value (1Gbps)
> by default when link speed is unset.
>
> Cc: Shyam Prasad N <nspmangalore@gmail.com>
> Cc: Tom Talpey <tom@talpey.com>
> Fixes: a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed")
> Reported-by: Frank Sorenson <sorenson@redhat.com>
> Reported-by: Jay Shin <jaeshin@redhat.com>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
> ---
>  fs/smb/client/smb2ops.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 87cb1872db28..9790ff2cc5b3 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -658,7 +658,8 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
>
>         while (bytes_left >= (ssize_t)sizeof(*p)) {
>                 memset(&tmp_iface, 0, sizeof(tmp_iface));
> -               tmp_iface.speed = le64_to_cpu(p->LinkSpeed);
> +               /* default to 1Gbps when link speed is unset */
> +               tmp_iface.speed = le64_to_cpu(p->LinkSpeed) ?: 1000000000;
>                 tmp_iface.rdma_capable = le32_to_cpu(p->Capability & RDMA_CAPABLE) ? 1 : 0;
>                 tmp_iface.rss_capable = le32_to_cpu(p->Capability & RSS_CAPABLE) ? 1 : 0;
>
> --
> 2.47.1
>
Steve French Jan. 18, 2025, 5:36 p.m. UTC | #2
With current for-next which includes this patch I noticed xfstest
cifs/104 now failing

[Sat Jan 18 00:17:13 2025] CIFS: Attempting to mount //win16.vm.test/Scratch
[Sat Jan 18 00:17:13 2025] CIFS: VFS: Error connecting to socket.
Aborting operation.
[Sat Jan 18 00:17:13 2025] CIFS: VFS: failed to open extra channel on
iface:fe80:0000:0000:0000:a435:84a7:df37:4e6b rc=-22
[Sat Jan 18 00:17:13 2025] CIFS: VFS: too many channel open attempts
(2 channels left to open)

http://smb311-linux-testing.southcentralus.cloudapp.azure.com/#/builders/3/builds/350/steps/26/logs/stdio

Thoughts?

On Thu, Jan 16, 2025 at 11:29 AM Paulo Alcantara <pc@manguebit.com> wrote:
>
> It isn't guaranteed that NETWORK_INTERFACE_INFO::LinkSpeed will always
> be set by the server, so the client must handle any values and then
> prevent oopses like below from happening:
>
> Oops: divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI
> CPU: 0 UID: 0 PID: 1323 Comm: cat Not tainted 6.13.0-rc7 #2
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-3.fc41
> 04/01/2014
> RIP: 0010:cifs_debug_data_proc_show+0xa45/0x1460 [cifs] Code: 00 00 48
> 89 df e8 3b cd 1b c1 41 f6 44 24 2c 04 0f 84 50 01 00 00 48 89 ef e8
> e7 d0 1b c1 49 8b 44 24 18 31 d2 49 8d 7c 24 28 <48> f7 74 24 18 48 89
> c3 e8 6e cf 1b c1 41 8b 6c 24 28 49 8d 7c 24
> RSP: 0018:ffffc90001817be0 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff88811230022c RCX: ffffffffc041bd99
> RDX: 0000000000000000 RSI: 0000000000000567 RDI: ffff888112300228
> RBP: ffff888112300218 R08: fffff52000302f5f R09: ffffed1022fa58ac
> R10: ffff888117d2c566 R11: 00000000fffffffe R12: ffff888112300200
> R13: 000000012a15343f R14: 0000000000000001 R15: ffff888113f2db58
> FS: 00007fe27119e740(0000) GS:ffff888148600000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fe2633c5000 CR3: 0000000124da0000 CR4: 0000000000750ef0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  ? __die_body.cold+0x19/0x27
>  ? die+0x2e/0x50
>  ? do_trap+0x159/0x1b0
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? do_error_trap+0x90/0x130
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? exc_divide_error+0x39/0x50
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? asm_exc_divide_error+0x1a/0x20
>  ? cifs_debug_data_proc_show+0xa39/0x1460 [cifs]
>  ? cifs_debug_data_proc_show+0xa45/0x1460 [cifs]
>  ? seq_read_iter+0x42e/0x790
>  seq_read_iter+0x19a/0x790
>  proc_reg_read_iter+0xbe/0x110
>  ? __pfx_proc_reg_read_iter+0x10/0x10
>  vfs_read+0x469/0x570
>  ? do_user_addr_fault+0x398/0x760
>  ? __pfx_vfs_read+0x10/0x10
>  ? find_held_lock+0x8a/0xa0
>  ? __pfx_lock_release+0x10/0x10
>  ksys_read+0xd3/0x170
>  ? __pfx_ksys_read+0x10/0x10
>  ? __rcu_read_unlock+0x50/0x270
>  ? mark_held_locks+0x1a/0x90
>  do_syscall_64+0xbb/0x1d0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fe271288911
> Code: 00 48 8b 15 01 25 10 00 f7 d8 64 89 02 b8 ff ff ff ff eb bd e8
> 20 ad 01 00 f3 0f 1e fa 80 3d b5 a7 10 00 00 74 13 31 c0 0f 05 <48> 3d
> 00 f0 ff ff 77 4f c3 66 0f 1f 44 00 00 55 48 89 e5 48 83 ec
> RSP: 002b:00007ffe87c079d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 0000000000040000 RCX: 00007fe271288911
> RDX: 0000000000040000 RSI: 00007fe2633c6000 RDI: 0000000000000003
> RBP: 00007ffe87c07a00 R08: 0000000000000000 R09: 00007fe2713e6380
> R10: 0000000000000022 R11: 0000000000000246 R12: 0000000000040000
> R13: 00007fe2633c6000 R14: 0000000000000003 R15: 0000000000000000
>  </TASK>
>
> Fix this by setting cifs_server_iface::speed to a sane value (1Gbps)
> by default when link speed is unset.
>
> Cc: Shyam Prasad N <nspmangalore@gmail.com>
> Cc: Tom Talpey <tom@talpey.com>
> Fixes: a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed")
> Reported-by: Frank Sorenson <sorenson@redhat.com>
> Reported-by: Jay Shin <jaeshin@redhat.com>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
> ---
>  fs/smb/client/smb2ops.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 87cb1872db28..9790ff2cc5b3 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -658,7 +658,8 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
>
>         while (bytes_left >= (ssize_t)sizeof(*p)) {
>                 memset(&tmp_iface, 0, sizeof(tmp_iface));
> -               tmp_iface.speed = le64_to_cpu(p->LinkSpeed);
> +               /* default to 1Gbps when link speed is unset */
> +               tmp_iface.speed = le64_to_cpu(p->LinkSpeed) ?: 1000000000;
>                 tmp_iface.rdma_capable = le32_to_cpu(p->Capability & RDMA_CAPABLE) ? 1 : 0;
>                 tmp_iface.rss_capable = le32_to_cpu(p->Capability & RSS_CAPABLE) ? 1 : 0;
>
> --
> 2.47.1
>
Paulo Alcantara Jan. 18, 2025, 6:16 p.m. UTC | #3
Steve French <smfrench@gmail.com> writes:

> With current for-next which includes this patch I noticed xfstest
> cifs/104 now failing

I can't find cifs/104 in xfstests.  What does this test do?
Steve French Jan. 18, 2025, 6:42 p.m. UTC | #4
It tests to make sure the number of expected channels is setup
(basically does a mount, and checks to make sure two connected
channels).  IIRC written by Ronnie

On Sat, Jan 18, 2025 at 12:16 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> Steve French <smfrench@gmail.com> writes:
>
> > With current for-next which includes this patch I noticed xfstest
> > cifs/104 now failing
>
> I can't find cifs/104 in xfstests.  What does this test do?
diff mbox series

Patch

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 87cb1872db28..9790ff2cc5b3 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -658,7 +658,8 @@  parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 
 	while (bytes_left >= (ssize_t)sizeof(*p)) {
 		memset(&tmp_iface, 0, sizeof(tmp_iface));
-		tmp_iface.speed = le64_to_cpu(p->LinkSpeed);
+		/* default to 1Gbps when link speed is unset */
+		tmp_iface.speed = le64_to_cpu(p->LinkSpeed) ?: 1000000000;
 		tmp_iface.rdma_capable = le32_to_cpu(p->Capability & RDMA_CAPABLE) ? 1 : 0;
 		tmp_iface.rss_capable = le32_to_cpu(p->Capability & RSS_CAPABLE) ? 1 : 0;