* [PATCH] cifs: ensure that cifs_get_root() only traverses directories
@ 2013-02-01 20:11 Jeff Layton
[not found] ` <1359749461-13458-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2013-02-01 20:11 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, afflux-7cISTWZqsW8RLla1DLYS2Q
Kjell Braden reported this oops:
[ 833.211970] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 833.212816] IP: [< (null)>] (null)
[ 833.213280] PGD 1b9b2067 PUD e9f7067 PMD 0
[ 833.213874] Oops: 0010 [#1] SMP
[ 833.214344] CPU 0
[ 833.214458] Modules linked in: des_generic md4 nls_utf8 cifs vboxvideo drm snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq bnep rfcomm snd_timer bluetooth snd_seq_device ppdev snd vboxguest parport_pc joydev mac_hid soundcore snd_page_alloc psmouse i2c_piix4 serio_raw lp parport usbhid hid e1000
[ 833.215629]
[ 833.215629] Pid: 1752, comm: mount.cifs Not tainted 3.0.0-rc7-bisectcifs-fec11dd9a0+ #18 innotek GmbH VirtualBox/VirtualBox
[ 833.215629] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
[ 833.215629] RSP: 0018:ffff8800119c9c50 EFLAGS: 00010282
[ 833.215629] RAX: ffffffffa02186c0 RBX: ffff88000c427780 RCX: 0000000000000000
[ 833.215629] RDX: 0000000000000000 RSI: ffff88000c427780 RDI: ffff88000c4362e8
[ 833.215629] RBP: ffff8800119c9c88 R08: ffff88001fc15e30 R09: 00000000d69515c7
[ 833.215629] R10: ffffffffa0201972 R11: ffff88000e8f6a28 R12: ffff88000c4362e8
[ 833.215629] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88001181aaa6
[ 833.215629] FS: 00007f2986171700(0000) GS:ffff88001fc00000(0000) knlGS:0000000000000000
[ 833.215629] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 833.215629] CR2: 0000000000000000 CR3: 000000001b982000 CR4: 00000000000006f0
[ 833.215629] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 833.215629] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 833.215629] Process mount.cifs (pid: 1752, threadinfo ffff8800119c8000, task ffff88001c1c16f0)
[ 833.215629] Stack:
[ 833.215629] ffffffff8116a9b5 ffff8800119c9c88 ffffffff81178075 0000000000000286
[ 833.215629] 0000000000000000 ffff88000c4276c0 ffff8800119c9ce8 ffff8800119c9cc8
[ 833.215629] ffffffff8116b06e ffff88001bc6fc00 ffff88000c4276c0 ffff88000c4276c0
[ 833.215629] Call Trace:
[ 833.215629] [<ffffffff8116a9b5>] ? d_alloc_and_lookup+0x45/0x90
[ 833.215629] [<ffffffff81178075>] ? d_lookup+0x35/0x60
[ 833.215629] [<ffffffff8116b06e>] __lookup_hash.part.14+0x9e/0xc0
[ 833.215629] [<ffffffff8116b1d6>] lookup_one_len+0x146/0x1e0
[ 833.215629] [<ffffffff815e4f7e>] ? _raw_spin_lock+0xe/0x20
[ 833.215629] [<ffffffffa01eef0d>] cifs_do_mount+0x26d/0x500 [cifs]
[ 833.215629] [<ffffffff81163bd3>] mount_fs+0x43/0x1b0
[ 833.215629] [<ffffffff8117d41a>] vfs_kern_mount+0x6a/0xd0
[ 833.215629] [<ffffffff8117e584>] do_kern_mount+0x54/0x110
[ 833.215629] [<ffffffff8117fdc2>] do_mount+0x262/0x840
[ 833.215629] [<ffffffff81108a0e>] ? __get_free_pages+0xe/0x50
[ 833.215629] [<ffffffff8117f9ca>] ? copy_mount_options+0x3a/0x180
[ 833.215629] [<ffffffff8118075d>] sys_mount+0x8d/0xe0
[ 833.215629] [<ffffffff815ece82>] system_call_fastpath+0x16/0x1b
[ 833.215629] Code: Bad RIP value.
[ 833.215629] RIP [< (null)>] (null)
[ 833.215629] RSP <ffff8800119c9c50>
[ 833.215629] CR2: 0000000000000000
[ 833.238525] ---[ end trace ec00758b8d44f529 ]---
When walking down the path on the server, it's possible to hit a
symlink. The path walking code assumes that the caller will handle that
situation properly, but cifs_get_root() isn't set up for it. This patch
prevents the oops by simply returning an error.
A better solution would be to try and chase the symlinks here, but that's
fairly complicated to handle.
Fixes:
https://bugzilla.kernel.org/show_bug.cgi?id=53221
Reported-and-Tested-by: Kjell Braden <afflux-7cISTWZqsW8RLla1DLYS2Q@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifsfs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index de7f916..e328339 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -558,6 +558,11 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
dentry = ERR_PTR(-ENOENT);
break;
}
+ if (!S_ISDIR(dir->i_mode)) {
+ dput(dentry);
+ dentry = ERR_PTR(-ENOTDIR);
+ break;
+ }
/* skip separators */
while (*s == sep)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cifs: ensure that cifs_get_root() only traverses directories
[not found] ` <1359749461-13458-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2013-02-02 5:33 ` Steve French
2013-02-03 16:49 ` Kjell Braden
1 sibling, 0 replies; 4+ messages in thread
From: Steve French @ 2013-02-02 5:33 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, afflux-7cISTWZqsW8RLla1DLYS2Q
Added cc: stable and merged to cifs-2.6.git
If no objections, seems like a reasonable candidate for 3.8 as it fixes an oops.
On Fri, Feb 1, 2013 at 2:11 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Kjell Braden reported this oops:
>
> [ 833.211970] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 833.212816] IP: [< (null)>] (null)
> [ 833.213280] PGD 1b9b2067 PUD e9f7067 PMD 0
> [ 833.213874] Oops: 0010 [#1] SMP
> [ 833.214344] CPU 0
> [ 833.214458] Modules linked in: des_generic md4 nls_utf8 cifs vboxvideo drm snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq bnep rfcomm snd_timer bluetooth snd_seq_device ppdev snd vboxguest parport_pc joydev mac_hid soundcore snd_page_alloc psmouse i2c_piix4 serio_raw lp parport usbhid hid e1000
> [ 833.215629]
> [ 833.215629] Pid: 1752, comm: mount.cifs Not tainted 3.0.0-rc7-bisectcifs-fec11dd9a0+ #18 innotek GmbH VirtualBox/VirtualBox
> [ 833.215629] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
> [ 833.215629] RSP: 0018:ffff8800119c9c50 EFLAGS: 00010282
> [ 833.215629] RAX: ffffffffa02186c0 RBX: ffff88000c427780 RCX: 0000000000000000
> [ 833.215629] RDX: 0000000000000000 RSI: ffff88000c427780 RDI: ffff88000c4362e8
> [ 833.215629] RBP: ffff8800119c9c88 R08: ffff88001fc15e30 R09: 00000000d69515c7
> [ 833.215629] R10: ffffffffa0201972 R11: ffff88000e8f6a28 R12: ffff88000c4362e8
> [ 833.215629] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88001181aaa6
> [ 833.215629] FS: 00007f2986171700(0000) GS:ffff88001fc00000(0000) knlGS:0000000000000000
> [ 833.215629] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 833.215629] CR2: 0000000000000000 CR3: 000000001b982000 CR4: 00000000000006f0
> [ 833.215629] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 833.215629] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 833.215629] Process mount.cifs (pid: 1752, threadinfo ffff8800119c8000, task ffff88001c1c16f0)
> [ 833.215629] Stack:
> [ 833.215629] ffffffff8116a9b5 ffff8800119c9c88 ffffffff81178075 0000000000000286
> [ 833.215629] 0000000000000000 ffff88000c4276c0 ffff8800119c9ce8 ffff8800119c9cc8
> [ 833.215629] ffffffff8116b06e ffff88001bc6fc00 ffff88000c4276c0 ffff88000c4276c0
> [ 833.215629] Call Trace:
> [ 833.215629] [<ffffffff8116a9b5>] ? d_alloc_and_lookup+0x45/0x90
> [ 833.215629] [<ffffffff81178075>] ? d_lookup+0x35/0x60
> [ 833.215629] [<ffffffff8116b06e>] __lookup_hash.part.14+0x9e/0xc0
> [ 833.215629] [<ffffffff8116b1d6>] lookup_one_len+0x146/0x1e0
> [ 833.215629] [<ffffffff815e4f7e>] ? _raw_spin_lock+0xe/0x20
> [ 833.215629] [<ffffffffa01eef0d>] cifs_do_mount+0x26d/0x500 [cifs]
> [ 833.215629] [<ffffffff81163bd3>] mount_fs+0x43/0x1b0
> [ 833.215629] [<ffffffff8117d41a>] vfs_kern_mount+0x6a/0xd0
> [ 833.215629] [<ffffffff8117e584>] do_kern_mount+0x54/0x110
> [ 833.215629] [<ffffffff8117fdc2>] do_mount+0x262/0x840
> [ 833.215629] [<ffffffff81108a0e>] ? __get_free_pages+0xe/0x50
> [ 833.215629] [<ffffffff8117f9ca>] ? copy_mount_options+0x3a/0x180
> [ 833.215629] [<ffffffff8118075d>] sys_mount+0x8d/0xe0
> [ 833.215629] [<ffffffff815ece82>] system_call_fastpath+0x16/0x1b
> [ 833.215629] Code: Bad RIP value.
> [ 833.215629] RIP [< (null)>] (null)
> [ 833.215629] RSP <ffff8800119c9c50>
> [ 833.215629] CR2: 0000000000000000
> [ 833.238525] ---[ end trace ec00758b8d44f529 ]---
>
> When walking down the path on the server, it's possible to hit a
> symlink. The path walking code assumes that the caller will handle that
> situation properly, but cifs_get_root() isn't set up for it. This patch
> prevents the oops by simply returning an error.
>
> A better solution would be to try and chase the symlinks here, but that's
> fairly complicated to handle.
>
> Fixes:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=53221
>
> Reported-and-Tested-by: Kjell Braden <afflux-7cISTWZqsW8RLla1DLYS2Q@public.gmane.org>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/cifsfs.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index de7f916..e328339 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -558,6 +558,11 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
> dentry = ERR_PTR(-ENOENT);
> break;
> }
> + if (!S_ISDIR(dir->i_mode)) {
> + dput(dentry);
> + dentry = ERR_PTR(-ENOTDIR);
> + break;
> + }
>
> /* skip separators */
> while (*s == sep)
> --
> 1.7.11.7
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cifs: ensure that cifs_get_root() only traverses directories
[not found] ` <1359749461-13458-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-02-02 5:33 ` Steve French
@ 2013-02-03 16:49 ` Kjell Braden
[not found] ` <510E9522.8040508-7cISTWZqsW8RLla1DLYS2Q@public.gmane.org>
1 sibling, 1 reply; 4+ messages in thread
From: Kjell Braden @ 2013-02-03 16:49 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
On 2013-02-01 21:11, Jeff Layton wrote:
> When walking down the path on the server, it's possible to hit a
> symlink. The path walking code assumes that the caller will handle that
> situation properly, but cifs_get_root() isn't set up for it. This patch
> prevents the oops by simply returning an error.
>
> A better solution would be to try and chase the symlinks here, but that's
> fairly complicated to handle.
Hi,
let me add that mounting paths below symlinks worked with the old path
walking before commit fec11dd9a0109fe52fd631e5c510778d6cbff6cc.
While it's not a big deal to work around this once the oops is fixed,
it would be great if the regression could be fixed.
--
Kjell
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cifs: ensure that cifs_get_root() only traverses directories
[not found] ` <510E9522.8040508-7cISTWZqsW8RLla1DLYS2Q@public.gmane.org>
@ 2013-02-03 16:54 ` Kjell Braden
0 siblings, 0 replies; 4+ messages in thread
From: Kjell Braden @ 2013-02-03 16:54 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
linux-cifs-u79uwXL29TY76Z2rM5mHXA
On 2013-02-03 17:49, Kjell Braden wrote:
> let me add that mounting paths below symlinks worked with the old path
> walking before commit fec11dd9a0109fe52fd631e5c510778d6cbff6cc.
Sorry, forgot specifics:
Consider the following tree:
/srv/symtest
/srv/symtest/dir
/srv/symtest/dir/subdir/
/srv/symtest/link -> dir/
/srv/symtest/abslink -> /
this works:
# mount.cifs //smbsrv/symtest/dir/subdir/ /mnt/
this causes oops but worked before fec11dd9a0:
# mount.cifs //smbsrv/symtest/link/subdir/ /mnt/
this fails with ENOTDIR, both before fec11dd9a0 and after:
# mount.cifs //smbsrv/symtest/abslink/srv/symtest/dir/subdir/ /mnt/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-02-03 16:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-01 20:11 [PATCH] cifs: ensure that cifs_get_root() only traverses directories Jeff Layton
[not found] ` <1359749461-13458-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-02-02 5:33 ` Steve French
2013-02-03 16:49 ` Kjell Braden
[not found] ` <510E9522.8040508-7cISTWZqsW8RLla1DLYS2Q@public.gmane.org>
2013-02-03 16:54 ` Kjell Braden
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.