Thanks for the quick patch, Jeff. I have just reproduced this again, so I'll try to test this patch to see how it goes. :) ----- Original Message ----- > From: "Jeff Layton" <jlayton@xxxxxxxxxx> > To: smfrench@xxxxxxxxx > Cc: caiqian@xxxxxxxxxx, linux-cifs@xxxxxxxxxxxxxxx > Sent: Wednesday, December 26, 2012 10:37:58 AM > Subject: [PATCH] cifs: move check for NULL socket into smb_send_rqst > > Cai reported this oops: > > [90701.616664] BUG: unable to handle kernel NULL pointer dereference > at 0000000000000028 > [90701.625438] IP: [<ffffffff814a343e>] kernel_setsockopt+0x2e/0x60 > [90701.632167] PGD fea319067 PUD 103fda4067 PMD 0 > [90701.637255] Oops: 0000 [#1] SMP > [90701.640878] Modules linked in: des_generic md4 nls_utf8 cifs > dns_resolver binfmt_misc tun sg igb iTCO_wdt iTCO_vendor_support > lpc_ich pcspkr i2c_i801 i2c_core i7core_edac edac_core ioatdma dca > mfd_core coretemp kvm_intel kvm crc32c_intel microcode sr_mod cdrom > ata_generic sd_mod pata_acpi crc_t10dif ata_piix libata megaraid_sas > dm_mirror dm_region_hash dm_log dm_mod > [90701.677655] CPU 10 > [90701.679808] Pid: 9627, comm: ls Tainted: G W 3.7.1+ #10 > QCI QSSC-S4R/QSSC-S4R > [90701.688950] RIP: 0010:[<ffffffff814a343e>] [<ffffffff814a343e>] > kernel_setsockopt+0x2e/0x60 > [90701.698383] RSP: 0018:ffff88177b431bb8 EFLAGS: 00010206 > [90701.704309] RAX: ffff88177b431fd8 RBX: 00007ffffffff000 RCX: > ffff88177b431bec > [90701.712271] RDX: 0000000000000003 RSI: 0000000000000006 RDI: > 0000000000000000 > [90701.720223] RBP: ffff88177b431bc8 R08: 0000000000000004 R09: > 0000000000000000 > [90701.728185] R10: 0000000000000001 R11: 0000000000000000 R12: > 0000000000000001 > [90701.736147] R13: ffff88184ef92000 R14: 0000000000000023 R15: > ffff88177b431c88 > [90701.744109] FS: 00007fd56a1a47c0(0000) GS:ffff88105fc40000(0000) > knlGS:0000000000000000 > [90701.753137] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > [90701.759550] CR2: 0000000000000028 CR3: 000000104f15f000 CR4: > 00000000000007e0 > [90701.767512] DR0: 0000000000000000 DR1: 0000000000000000 DR2: > 0000000000000000 > [90701.775465] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: > 0000000000000400 > [90701.783428] Process ls (pid: 9627, threadinfo ffff88177b430000, > task ffff88185ca4cb60) > [90701.792261] Stack: > [90701.794505] 0000000000000023 ffff88177b431c50 ffff88177b431c38 > ffffffffa014fcb1 > [90701.802809] ffff88184ef921bc 0000000000000000 00000001ffffffff > ffff88184ef921c0 > [90701.811123] ffff88177b431c08 ffffffff815ca3d9 ffff88177b431c18 > ffff880857758000 > [90701.819433] Call Trace: > [90701.822183] [<ffffffffa014fcb1>] smb_send_rqst+0x71/0x1f0 [cifs] > [90701.828991] [<ffffffff815ca3d9>] ? schedule+0x29/0x70 > [90701.834736] [<ffffffffa014fe6d>] smb_sendv+0x3d/0x40 [cifs] > [90701.841062] [<ffffffffa014fe96>] smb_send+0x26/0x30 [cifs] > [90701.847291] [<ffffffffa015801f>] send_nt_cancel+0x6f/0xd0 [cifs] > [90701.854102] [<ffffffffa015075e>] SendReceive+0x18e/0x360 [cifs] > [90701.860814] [<ffffffffa0134a78>] CIFSFindFirst+0x1a8/0x3f0 [cifs] > [90701.867724] [<ffffffffa013f731>] ? > build_path_from_dentry+0xf1/0x260 [cifs] > [90701.875601] [<ffffffffa013f731>] ? > build_path_from_dentry+0xf1/0x260 [cifs] > [90701.883477] [<ffffffffa01578e6>] cifs_query_dir_first+0x26/0x30 > [cifs] > [90701.890869] [<ffffffffa015480d>] initiate_cifs_search+0xed/0x250 > [cifs] > [90701.898354] [<ffffffff81195970>] ? fillonedir+0x100/0x100 > [90701.904486] [<ffffffffa01554cb>] cifs_readdir+0x45b/0x8f0 [cifs] > [90701.911288] [<ffffffff81195970>] ? fillonedir+0x100/0x100 > [90701.917410] [<ffffffff81195970>] ? fillonedir+0x100/0x100 > [90701.923533] [<ffffffff81195970>] ? fillonedir+0x100/0x100 > [90701.929657] [<ffffffff81195848>] vfs_readdir+0xb8/0xe0 > [90701.935490] [<ffffffff81195b9f>] sys_getdents+0x8f/0x110 > [90701.941521] [<ffffffff815d3b99>] system_call_fastpath+0x16/0x1b > [90701.948222] Code: 66 90 55 65 48 8b 04 25 f0 c6 00 00 48 89 e5 53 > 48 83 ec 08 83 fe 01 48 8b 98 48 e0 ff ff 48 c7 80 48 e0 ff ff ff ff > ff ff 74 22 <48> 8b 47 28 ff 50 68 65 48 8b 14 25 f0 c6 00 00 48 89 > 9a 48 e0 > [90701.970313] RIP [<ffffffff814a343e>] kernel_setsockopt+0x2e/0x60 > [90701.977125] RSP <ffff88177b431bb8> > [90701.981018] CR2: 0000000000000028 > [90701.984809] ---[ end trace 24bd602971110a43 ]--- > > This is likely due to a race vs. a reconnection event. > > The current code checks for a NULL socket in smb_send_kvec, but > that's > too late. By the time that check is done, the socket will already > have > been passed to kernel_setsockopt. Move the check into smb_send_rqst, > so > that it's checked earlier. > > In truth, this is a bit of a half-assed fix. The -ENOTSOCK error > return here looks like it could bubble back up to userspace. The > locking > rules around the ssocket pointer are really unclear as well. There > are > cases where the ssocket pointer is changed without holding the > srv_mutex, > but I'm not clear whether there's a potential race here yet or not. > > This code seems like it could benefit from some fundamental re-think > of > how the socket handling should behave. Until then though, this patch > should at least fix the above oops in most cases. > > Cc: <stable@xxxxxxxxxxxxxxx> # 3.7+ > Reported-by: CAI Qian <caiqian@xxxxxxxxxx> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > fs/cifs/transport.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > index 0ed7bc2..3e3b19f 100644 > --- a/fs/cifs/transport.c > +++ b/fs/cifs/transport.c > @@ -144,9 +144,6 @@ smb_send_kvec(struct TCP_Server_Info *server, > struct kvec *iov, size_t n_vec, > > *sent = 0; > > - if (ssocket == NULL) > - return -ENOTSOCK; /* BB eventually add reconnect code here */ > - > smb_msg.msg_name = (struct sockaddr *) &server->dstaddr; > smb_msg.msg_namelen = sizeof(struct sockaddr); > smb_msg.msg_control = NULL; > @@ -291,6 +288,9 @@ smb_send_rqst(struct TCP_Server_Info *server, > struct smb_rqst *rqst) > struct socket *ssocket = server->ssocket; > int val = 1; > > + if (ssocket == NULL) > + return -ENOTSOCK; > + > cFYI(1, "Sending smb: smb_len=%u", smb_buf_length); > dump_smb(iov[0].iov_base, iov[0].iov_len); > > -- > 1.7.11.7 > > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
