* Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
@ 2020-03-24 21:06 Qian Cai
2020-03-24 21:46 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Qian Cai @ 2020-03-24 21:06 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, linux-fsdevel, LKML
Reverted the series on the top of today's linux-next fixed boot crashes.
# git revert 609c56723133..e0e25e9bbed5 --no-edit [1]
[ 53.027443][ T3519] BUG: Kernel NULL pointer dereference on read at 0x00000000
[ 53.027480][ T3519] Faulting instruction address: 0xc0000000004dbfa4
[ 53.027498][ T3519] Oops: Kernel access of bad area, sig: 11 [#1]
[ 53.027521][ T3519] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
[ 53.027538][ T3519] Modules linked in: kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci mdio libata tg3 libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
[ 53.027594][ T3519] CPU: 36 PID: 3519 Comm: polkitd Not tainted 5.6.0-rc7-next-20200324 #1
[ 53.027618][ T3519] NIP: c0000000004dbfa4 LR: c0000000004dc040 CTR: 0000000000000000
[ 53.027634][ T3519] REGS: c0002013879af810 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200324)
[ 53.027668][ T3519] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 24004422 XER: 20040000
[ 53.027708][ T3519] CFAR: c0000000004dc044 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
[ 53.027708][ T3519] GPR00: c0000000004dc040 c0002013879afaa0 c00000000165a500 0000000000000000
[ 53.027708][ T3519] GPR04: c000000001511408 0000000000000000 c0002013879af834 0000000000000002
[ 53.027708][ T3519] GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000001
[ 53.027708][ T3519] GPR12: 0000000000004000 c000001ffffe1e00 0000000000000000 0000000000000000
[ 53.027708][ T3519] GPR16: 0000000000000000 0000000000000001 0000000000000000 0000000000000000
[ 53.027708][ T3519] GPR20: c000200ea1eacf38 c000201c8102f043 2f2f2f2f2f2f2f2f 0000000000000003
[ 53.027708][ T3519] GPR24: 0000000000000000 c0002013879afbc8 fffffffffffff000 0000000000200000
[ 53.027708][ T3519] GPR28: ffffffffffffffff 61c8864680b583eb 0000000000000000 0000000000002e2e
[ 53.027931][ T3519] NIP [c0000000004dbfa4] link_path_walk+0x284/0x4c0
__d_entry_type at include/linux/dcache.h:389
(inlined by) d_can_lookup at include/linux/dcache.h:404
(inlined by) link_path_walk at fs/namei.c:2178
[ 53.027963][ T3519] LR [c0000000004dc040] link_path_walk+0x320/0x4c0
[ 53.027993][ T3519] Call Trace:
[ 53.028013][ T3519] [c0002013879afaa0] [c0000000004dc040] link_path_walk+0x320/0x4c0 (unreliable)
[ 53.028050][ T3519] [c0002013879afb60] [c0000000004dc334] path_lookupat+0x94/0x1b0
[ 53.028084][ T3519] [c0002013879afba0] [c0000000004ddf80] filename_lookup.part.55+0xa0/0x170
[ 53.028101][ T3519] [c0002013879afce0] [c0000000004ca748] vfs_statx+0xa8/0x190
[ 53.028117][ T3519] [c0002013879afd60] [c0000000004cacc0] __do_sys_newstat+0x40/0x90
[ 53.028145][ T3519] [c0002013879afe20] [c00000000000b378] system_call+0x5c/0x68
[ 53.028178][ T3519] Instruction dump:
[ 53.028197][ T3519] 3bdeffff e9390058 38800000 7f23cb78 7fde07b4 1d5e0030 7d295214 eaa90020
[ 53.028245][ T3519] 4bfffac5 2fa30000 409e00ac e9390008 <81290000> 55290256 7f89d800 409e0160
[ 53.028284][ T3519] ---[ end trace 0effae07d5cccfa0 ]—
[ 705.047353][ T4874] BUG: KASAN: invalid-access in link_path_walk+0x374/0x53c
__d_entry_type at include/linux/dcache.h:389
(inlined by) d_can_lookup at include/linux/dcache.h:404
(inlined by) link_path_walk at fs/namei.c:2178
[ 705.054422][ T4874] Read of size 4 at addr 0000000000000000 by task plymouthd/4874
[ 705.062003][ T4874]
[ 705.064213][ T4874] CPU: 16 PID: 4874 Comm: plymouthd Tainted: G L 5.6.0-rc7-next-20200324 #1
[ 705.074055][ T4874] Hardware name: HPE Apollo 70 /C01_APACHE_MB , BIOS L50_5.13_1.11 06/18/2019
[ 705.084502][ T4874] Call trace:
[ 705.087663][ T4874] dump_backtrace+0x0/0x224
[ 705.092036][ T4874] show_stack+0x20/0x2c
[ 705.096063][ T4874] dump_stack+0xfc/0x184
[ 705.100178][ T4874] __kasan_report+0x178/0x238
[ 705.104725][ T4874] kasan_report+0x3c/0x58
[ 705.108925][ T4874] check_memory_region+0x98/0xa0
[ 705.113734][ T4874] __hwasan_load4_noabort+0x18/0x20
[ 705.118801][ T4874] link_path_walk+0x374/0x53c
[ 705.123350][ T4874] path_lookupat+0x78/0x1d4
[ 705.127723][ T4874] filename_lookup+0x80/0x124
[ 705.132270][ T4874] user_path_at_empty+0x54/0x68
[ 705.136990][ T4874] vfs_statx+0xcc/0x1b8
[ 705.141016][ T4874] __arm64_sys_newfstatat+0x94/0x120
[ 705.146169][ T4874] do_el0_svc+0x128/0x1dc
[ 705.150369][ T4874] el0_sync_handler+0xd0/0x268
[ 705.155003][ T4874] el0_sync+0x164/0x180
[ 705.159028][ T4874] ==================================================================
[ 705.166957][ T4874] Disabling lock debugging due to kernel taint
[ 705.173067][ T4874] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 705.182599][ T4874] Mem abort info:
[ 705.186104][ T4874] ESR = 0x96000005
[ 705.189906][ T4874] EC = 0x25: DABT (current EL), IL = 32 bits
[ 705.195928][ T4874] SET = 0, FnV = 0
[ 705.199727][ T4874] EA = 0, S1PTW = 0
[ 705.203578][ T4874] Data abort info:
[ 705.207168][ T4874] ISV = 0, ISS = 0x00000005
[ 705.211749][ T4874] CM = 0, WnR = 0
[ 705.215431][ T4874] user pgtable: 64k pages, 48-bit VAs, pgdp=0000009659f42000
[ 705.222702][ T4874] [0000000000000000] pgd=0000000000000000, pud=0000000000000000
[ 705.230250][ T4874] Internal error: Oops: 96000005 [#1] SMP
[ 705.235824][ T4874] Modules linked in: thunderx2_pmu processor efivarfs ip_tables xfs libcrc32c sd_mod ahci libahci mlx5_core libata dm_mirror dm_region_hash dm_log dm_mod
[ 705.251173][ T4874] CPU: 16 PID: 4874 Comm: plymouthd Tainted: G B L 5.6.0-rc7-next-20200324 #1
[ 705.260999][ T4874] Hardware name: HPE Apollo 70 /C01_APACHE_MB , BIOS L50_5.13_1.11 06/18/2019
[ 705.271438][ T4874] pstate: 60400009 (nZCv daif +PAN -UAO BTYPE=--)
[ 705.277708][ T4874] pc : link_path_walk+0x374/0x53c
[ 705.282587][ T4874] lr : link_path_walk+0x374/0x53c
[ 705.287463][ T4874] sp : b1ff00916cdefa90
[ 705.291473][ T4874] x29: b1ff00916cdefb30 x28: 9cff00098d5eb703
[ 705.297485][ T4874] x27: 0000000000000000 x26: fefefefefefefeff
[ 705.303496][ T4874] x25: 0000000236266748 x24: 2f2f2f2f2f2f2f2f
[ 705.309507][ T4874] x23: b1ff00916cdefba0 x22: b1ff00916cdefbc8
[ 705.315518][ T4874] x21: b1ff00916cdefbe0 x20: b1ff00916cdefbd0
[ 705.321529][ T4874] x19: b1ff00916cdefb98 x18: 0000000000000000
[ 705.327540][ T4874] x17: 0000000000000000 x16: 0000000000000000
[ 705.333550][ T4874] x15: 0000000000000000 x14: 2020202020202020
[ 705.339561][ T4874] x13: 20424d5f45484341 x12: 50415f3130432f20
[ 705.345571][ T4874] x11: 0000000000000003 x10: ffff8008bb246a3e
[ 705.351582][ T4874] x9 : 68bdf6118cf10200 x8 : 0000000000000000
[ 705.357592][ T4874] x7 : aaaaaaaaaaaaaaaa x6 : 0000000000000000
[ 705.363602][ T4874] x5 : 0000000000000080 x4 : 0000000000000000
[ 705.369612][ T4874] x3 : ffff900010a5a394 x2 : 0000000000000001
[ 705.375622][ T4874] x1 : 0000000000000004 x0 : 0000000000000000
[ 705.381631][ T4874] Call trace:
[ 705.384777][ T4874] link_path_walk+0x374/0x53c
[ 705.389311][ T4874] path_lookupat+0x78/0x1d4
[ 705.393670][ T4874] filename_lookup+0x80/0x124
[ 705.398204][ T4874] user_path_at_empty+0x54/0x68
[ 705.402909][ T4874] vfs_statx+0xcc/0x1b8
[ 705.406921][ T4874] __arm64_sys_newfstatat+0x94/0x120
[ 705.412060][ T4874] do_el0_svc+0x128/0x1dc
[ 705.416247][ T4874] el0_sync_handler+0xd0/0x268
[ 705.420865][ T4874] el0_sync+0x164/0x180
[ 705.424883][ T4874] Code: 97fe39bd f94002fb aa1b03e0 97fe39aa (b9400368)
[ 705.432066][ T4874] ---[ end trace 71f0365c08ac491a ]---
[ 705.437381][ T4874] Kernel panic - not syncing: Fatal exception
[ 705.443608][ T4874] SMP: stopping secondary CPUs
[ 705.448297][ T4874] Kernel Offset: disabled
[ 705.452483][ T4874] CPU features: 0x006002,61000c38
[ 705.457359][ T4874] Memory Limit: none
[ 705.461411][ T4874] ---[ end Kernel panic - not syncing: Fatal exception ]—
[1]
e0e25e9bbed5 lookup_open(): don't bother with fallbacks to lookup+create
b686da54700f atomic_open(): no need to pass struct open_flags anymore
60e1d0b8512f open_last_lookups(): move complete_walk() into do_open()
4d7ed93ff9db open_last_lookups(): lift O_EXCL|O_CREAT handling into do_open()
57e9b028e9e7 open_last_lookups(): don't abuse complete_walk() when all we want is unlazy
c01d40b1c03c open_last_lookups(): consolidate fsnotify_create() calls
c8291f6b0037 take post-lookup part of do_last() out of loop
881386f7e46a link_path_walk(): sample parent's i_uid and i_mode for the last component
0e47dacb7f29 __nd_alloc_stack(): make it return bool
794dc2d56401 reserve_stack(): switch to __nd_alloc_stack()
59089811438c pick_link(): take reserving space on stack into a new helper
8c60edbc56a2 pick_link(): more straightforward handling of allocation failures
4efc770ddf45 fold path_to_nameidata() into its only remaining caller
dcc11116def1 pick_link(): pass it struct path already with normal refcounting rules
0058fcb4c3b5 fs/namei.c: kill follow_mount()
ffa2db4ac3e7 non-RCU analogue of the previous commit
8255cecd93ba helper for mount rootwards traversal
573f88cea0e2 follow_dotdot(): be lazy about changing nd->path
ea63a0dc31fd follow_dotdot_rcu(): be lazy about changing nd->path
5c19a79cd9d3 follow_dotdot{,_rcu}(): massage loops
5e3c3570ec97 lift all calls of step_into() out of follow_dotdot/follow_dotdot_rcu
6dfd9fe54dfd follow_dotdot{,_rcu}(): switch to use of step_into()
7521f22b3ce2 handle_dots(), follow_dotdot{,_rcu}(): preparation to switch to step_into()
957dd41d8842 move handle_dots(), follow_dotdot() and follow_dotdot_rcu() past step_into()
c9a0f75d81e3 follow_dotdot{,_rcu}(): lift LOOKUP_BENEATH checks out of loop
abc2c632e0ce follow_dotdot{,_rcu}(): lift switching nd->path to parent out of loop
a6a7eb7628cf expand path_parent_directory() in its callers
63b27720a476 path_parent_directory(): leave changing path->dentry to callers
6b03f7edf43e path_connected(): pass mount and dentry separately
c981a4828125 split the lookup-related parts of do_last() into a separate helper
973d4b73fbaf do_last(): rejoin the common path even earlier in FMODE_{OPENED,CREATED} case
8795e7d48288 do_last(): simplify the liveness analysis past finish_open_created
5a2d3edd8dad do_last(): rejoing the common path earlier in FMODE_{OPENED,CREATED} case
59e96e65833e do_last(): don't bother with keeping got_write in FMODE_OPENED case
3ad5615a071f do_last(): merge the may_open() calls
7be219b4dcd9 atomic_open(): lift the call of may_open() into do_last()
6fb968cdf9d0 atomic_open(): return the right dentry in FMODE_OPENED case
9deed3ebca24 new helper: traverse_mounts()
ea936aeb3ead massage __follow_mount_rcu() a bit
c108837e06b6 namei: have link_path_walk() maintain LOOKUP_PARENT
d8d4611a4f2d link_path_walk(): simplify stack handling
b1a819724074 pick_link(): check for WALK_TRAILING, not LOOKUP_PARENT
8c4efe22e7c4 namei: invert the meaning of WALK_FOLLOW
b4c0353693d2 sanitize handling of nd->last_type, kill LAST_BIND
ad6cc4c338f4 finally fold get_link() into pick_link()
06708adb99e8 merging pick_link() with get_link(), part 6
b0417d2c7298 merging pick_link() with get_link(), part 5
92d270165cff merging pick_link() with get_link(), part 4
40fcf5a931af merging pick_link() with get_link(), part 3
1ccac622f9da merging pick_link() with get_link(), part 2
43679723d27f merging pick_link() with get_link(), part 1
a9dc1494a782 expand the only remaining call of path_lookup_conditional()
161aff1d93ab LOOKUP_MOUNTPOINT: fold path_mountpointat() into path_lookupat()
cbae4d12eeee fold handle_mounts() into step_into()
aca2903eefd0 new step_into() flag: WALK_NOFOLLOW
56676ec39019 step_into() callers: dismiss the symlink earlier
20e343571cef lookup_fast(): take mount traversal into callers
c153007b7b7a teach handle_mounts() to handle RCU mode
b023e1728bec lookup_fast(): consolidate the RCU success case
db3c9ade50b1 handle_mounts(): pass dentry in, turn path into a pure out argument
e73cabff5917 do_last(): collapse the call of path_to_nameidata()
da5ebf5aa676 lookup_open(): saner calling conventions (return dentry on success)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-24 21:06 Null-ptr-deref due to "sanitized pathwalk machinery (v4)" Qian Cai
@ 2020-03-24 21:46 ` Al Viro
2020-03-25 1:49 ` Qian Cai
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2020-03-24 21:46 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Tue, Mar 24, 2020 at 05:06:03PM -0400, Qian Cai wrote:
> Reverted the series on the top of today's linux-next fixed boot crashes.
Umm... How about a reproducer (or bisect of vfs.git#work.dotdot, assuming
it reproduces there)?
> [ 53.027443][ T3519] BUG: Kernel NULL pointer dereference on read at 0x00000000
> [ 53.027480][ T3519] Faulting instruction address: 0xc0000000004dbfa4
> [ 53.027498][ T3519] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 53.027521][ T3519] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
> [ 53.027538][ T3519] Modules linked in: kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci mdio libata tg3 libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
> [ 53.027594][ T3519] CPU: 36 PID: 3519 Comm: polkitd Not tainted 5.6.0-rc7-next-20200324 #1
> [ 53.027618][ T3519] NIP: c0000000004dbfa4 LR: c0000000004dc040 CTR: 0000000000000000
> [ 53.027634][ T3519] REGS: c0002013879af810 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200324)
> [ 53.027668][ T3519] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 24004422 XER: 20040000
> [ 53.027708][ T3519] CFAR: c0000000004dc044 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
> [ 53.027708][ T3519] GPR00: c0000000004dc040 c0002013879afaa0 c00000000165a500 0000000000000000
> [ 53.027708][ T3519] GPR04: c000000001511408 0000000000000000 c0002013879af834 0000000000000002
> [ 53.027708][ T3519] GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000001
> [ 53.027708][ T3519] GPR12: 0000000000004000 c000001ffffe1e00 0000000000000000 0000000000000000
> [ 53.027708][ T3519] GPR16: 0000000000000000 0000000000000001 0000000000000000 0000000000000000
> [ 53.027708][ T3519] GPR20: c000200ea1eacf38 c000201c8102f043 2f2f2f2f2f2f2f2f 0000000000000003
> [ 53.027708][ T3519] GPR24: 0000000000000000 c0002013879afbc8 fffffffffffff000 0000000000200000
> [ 53.027708][ T3519] GPR28: ffffffffffffffff 61c8864680b583eb 0000000000000000 0000000000002e2e
> [ 53.027931][ T3519] NIP [c0000000004dbfa4] link_path_walk+0x284/0x4c0
> __d_entry_type at include/linux/dcache.h:389
> (inlined by) d_can_lookup at include/linux/dcache.h:404
> (inlined by) link_path_walk at fs/namei.c:2178
... and apparently NULL nd->path.dentry there. After walk_component()
having returned NULL. Which means either handle_dots() returning NULL
or step_into() doing the same. The former means either (for "..")
step_into() having returned NULL, or nd->path.dentry left unchanged.
So we either have step_into() returning NULL with nd->path.dentry ending up
NULL, or we'd entered link_path_walk() with nd->path.dentry being NULL (it
must have been that way on the entry, or we would've barfed on the previous
iteration).
1) step_into() returns NULL either after
if (likely(!d_is_symlink(path.dentry)) ||
((flags & WALK_TRAILING) && !(nd->flags & LOOKUP_FOLLOW)) ||
(flags & WALK_NOFOLLOW)) {
/* not a symlink or should not follow */
if (!(nd->flags & LOOKUP_RCU)) {
dput(nd->path.dentry);
if (nd->path.mnt != path.mnt)
mntput(nd->path.mnt);
}
nd->path = path;
nd->inode = inode;
nd->seq = seq;
return NULL;
in which case nd->path.dentry is left equal to path.dentry, which can't be
NULL (we would've oopsed on d_is_symlink() if it had been) or it's
pick_link() returning NULL and leaving NULL nd->path.dentry.
pick_link() either leaves nd->path unchanged (in which case we are back to
the "had NULL nd->path.dentry on entry into link_path_walk()") or does
nd_jump_root() (absolute symlinks) or has ->get_link() call nd_jump_link().
nd_jump_root() cannot survive leaving NULL in ->path.dentry - it hits
either
d = nd->path.dentry;
nd->inode = d->d_inode;
or
nd->inode = nd->path.dentry->d_inode;
and either would've ooped right there.
nd_jump_link() hits
nd->inode = nd->path.dentry->d_inode;
on the way out, which also should be impossible to survive.
So we appear to have hit link_path_walk() with NULL nd->path.dentry. And
it's path_lookupat() from vfs_statx(), so we don't have LOOKUP_DOWN there.
Which means either path_init() leaving NULL nd->path.dentry or lookup_last()
returning NULL and leaving NULL nd->path.dentry... The latter is basically
walk_component(), so we would've had left link_path_walk() without an
error, with symlink picked and with NULL nd->path.dentry. Which means
having the previous call of link_path_walk() also entered with NULL
nd->path.dentry...
OK, so it looks like path_init() returning a string and leaving that...
And I don't see any way for that to happen...
Right, so... Could you slap the following
if (WARN_ON(!nd->path.dentry))
printk(KERN_ERR "pathname = %s\n", nd->name->name);
1) into beginning of link_path_walk(), right before
while (*name=='/')
name++;
if (!*name)
return 0;
in there.
2) into pick_link(), right after
all_done: // pure jump
and see what your reproducer catches?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-24 21:46 ` Al Viro
@ 2020-03-25 1:49 ` Qian Cai
2020-03-25 2:13 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Qian Cai @ 2020-03-25 1:49 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, linux-fsdevel, LKML
> On Mar 24, 2020, at 5:46 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Mar 24, 2020 at 05:06:03PM -0400, Qian Cai wrote:
>> Reverted the series on the top of today's linux-next fixed boot crashes.
>
> Umm... How about a reproducer (or bisect of vfs.git#work.dotdot, assuming
> it reproduces there)?
Booting powerpc (power9 PowerNV) and arm64 (HPE Apollo 70, Thunder X2) a
few times could trigger it using the configs here,
https://github.com/cailca/linux-mm
>
>> [ 53.027443][ T3519] BUG: Kernel NULL pointer dereference on read at 0x00000000
>> [ 53.027480][ T3519] Faulting instruction address: 0xc0000000004dbfa4
>> [ 53.027498][ T3519] Oops: Kernel access of bad area, sig: 11 [#1]
>> [ 53.027521][ T3519] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
>> [ 53.027538][ T3519] Modules linked in: kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci mdio libata tg3 libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
>> [ 53.027594][ T3519] CPU: 36 PID: 3519 Comm: polkitd Not tainted 5.6.0-rc7-next-20200324 #1
>> [ 53.027618][ T3519] NIP: c0000000004dbfa4 LR: c0000000004dc040 CTR: 0000000000000000
>> [ 53.027634][ T3519] REGS: c0002013879af810 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200324)
>> [ 53.027668][ T3519] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 24004422 XER: 20040000
>> [ 53.027708][ T3519] CFAR: c0000000004dc044 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
>> [ 53.027708][ T3519] GPR00: c0000000004dc040 c0002013879afaa0 c00000000165a500 0000000000000000
>> [ 53.027708][ T3519] GPR04: c000000001511408 0000000000000000 c0002013879af834 0000000000000002
>> [ 53.027708][ T3519] GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000001
>> [ 53.027708][ T3519] GPR12: 0000000000004000 c000001ffffe1e00 0000000000000000 0000000000000000
>> [ 53.027708][ T3519] GPR16: 0000000000000000 0000000000000001 0000000000000000 0000000000000000
>> [ 53.027708][ T3519] GPR20: c000200ea1eacf38 c000201c8102f043 2f2f2f2f2f2f2f2f 0000000000000003
>> [ 53.027708][ T3519] GPR24: 0000000000000000 c0002013879afbc8 fffffffffffff000 0000000000200000
>> [ 53.027708][ T3519] GPR28: ffffffffffffffff 61c8864680b583eb 0000000000000000 0000000000002e2e
>> [ 53.027931][ T3519] NIP [c0000000004dbfa4] link_path_walk+0x284/0x4c0
>> __d_entry_type at include/linux/dcache.h:389
>> (inlined by) d_can_lookup at include/linux/dcache.h:404
>> (inlined by) link_path_walk at fs/namei.c:2178
>
> ... and apparently NULL nd->path.dentry there. After walk_component()
> having returned NULL. Which means either handle_dots() returning NULL
> or step_into() doing the same. The former means either (for "..")
> step_into() having returned NULL, or nd->path.dentry left unchanged.
>
> So we either have step_into() returning NULL with nd->path.dentry ending up
> NULL, or we'd entered link_path_walk() with nd->path.dentry being NULL (it
> must have been that way on the entry, or we would've barfed on the previous
> iteration).
>
> 1) step_into() returns NULL either after
> if (likely(!d_is_symlink(path.dentry)) ||
> ((flags & WALK_TRAILING) && !(nd->flags & LOOKUP_FOLLOW)) ||
> (flags & WALK_NOFOLLOW)) {
> /* not a symlink or should not follow */
> if (!(nd->flags & LOOKUP_RCU)) {
> dput(nd->path.dentry);
> if (nd->path.mnt != path.mnt)
> mntput(nd->path.mnt);
> }
> nd->path = path;
> nd->inode = inode;
> nd->seq = seq;
> return NULL;
> in which case nd->path.dentry is left equal to path.dentry, which can't be
> NULL (we would've oopsed on d_is_symlink() if it had been) or it's
> pick_link() returning NULL and leaving NULL nd->path.dentry.
>
> pick_link() either leaves nd->path unchanged (in which case we are back to
> the "had NULL nd->path.dentry on entry into link_path_walk()") or does
> nd_jump_root() (absolute symlinks) or has ->get_link() call nd_jump_link().
> nd_jump_root() cannot survive leaving NULL in ->path.dentry - it hits
> either
> d = nd->path.dentry;
> nd->inode = d->d_inode;
> or
> nd->inode = nd->path.dentry->d_inode;
> and either would've ooped right there.
> nd_jump_link() hits
> nd->inode = nd->path.dentry->d_inode;
> on the way out, which also should be impossible to survive.
>
> So we appear to have hit link_path_walk() with NULL nd->path.dentry. And
> it's path_lookupat() from vfs_statx(), so we don't have LOOKUP_DOWN there.
> Which means either path_init() leaving NULL nd->path.dentry or lookup_last()
> returning NULL and leaving NULL nd->path.dentry... The latter is basically
> walk_component(), so we would've had left link_path_walk() without an
> error, with symlink picked and with NULL nd->path.dentry. Which means
> having the previous call of link_path_walk() also entered with NULL
> nd->path.dentry...
>
> OK, so it looks like path_init() returning a string and leaving that...
> And I don't see any way for that to happen...
>
>
> Right, so... Could you slap the following
> if (WARN_ON(!nd->path.dentry))
> printk(KERN_ERR "pathname = %s\n", nd->name->name);
> 1) into beginning of link_path_walk(), right before
> while (*name=='/')
> name++;
> if (!*name)
> return 0;
> in there.
> 2) into pick_link(), right after
> all_done: // pure jump
>
> and see what your reproducer catches?
It does not catch anything at all with the patch,
diff --git a/fs/namei.c b/fs/namei.c
index 311e33dbac63..d3ab3dd3e522 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1659,6 +1659,8 @@ static const char *pick_link(struct nameidata *nd, struct path *link,
if (*res)
return res;
all_done: // pure jump
+if (WARN_ON(!nd->path.dentry))
+ printk(KERN_ERR "pathname = %s\n", nd->name->name);
put_link(nd);
return NULL;
}
@@ -2096,6 +2098,8 @@ static int link_path_walk(const char *name, struct nameidata *nd)
nd->flags |= LOOKUP_PARENT;
if (IS_ERR(name))
return PTR_ERR(name);
+if (WARN_ON(!nd->path.dentry))
+ printk(KERN_ERR "pathname = %s\n", nd->name->name);
while (*name=='/')
name++;
if (!*name)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 1:49 ` Qian Cai
@ 2020-03-25 2:13 ` Al Viro
2020-03-25 3:24 ` Qian Cai
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2020-03-25 2:13 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Tue, Mar 24, 2020 at 09:49:48PM -0400, Qian Cai wrote:
> It does not catch anything at all with the patch,
You mean, oops happens, but neither WARN_ON() is triggered?
Lovely... Just to make sure: could you slap the same couple
of lines just before
if (unlikely(!d_can_lookup(nd->path.dentry))) {
in link_path_walk(), just to check if I have misread the trace
you've got?
Does that (+ other two inserts) end up with
1) some of these WARN_ON() triggered when oops happens or
2) oops is happening, but neither WARN_ON() triggers or
3) oops not happening / becoming harder to hit?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 2:13 ` Al Viro
@ 2020-03-25 3:24 ` Qian Cai
2020-03-25 4:03 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Qian Cai @ 2020-03-25 3:24 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, linux-fsdevel, LKML
> On Mar 24, 2020, at 10:13 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Mar 24, 2020 at 09:49:48PM -0400, Qian Cai wrote:
>
>> It does not catch anything at all with the patch,
>
> You mean, oops happens, but neither WARN_ON() is triggered?
> Lovely... Just to make sure: could you slap the same couple
> of lines just before
> if (unlikely(!d_can_lookup(nd->path.dentry))) {
> in link_path_walk(), just to check if I have misread the trace
> you've got?
>
> Does that (+ other two inserts) end up with
> 1) some of these WARN_ON() triggered when oops happens or
> 2) oops is happening, but neither WARN_ON() triggers or
> 3) oops not happening / becoming harder to hit?
Only the one just before
if (unlikely(!d_can_lookup(nd->path.dentry))) {
In link_path_walk() will trigger.
[ 245.766216][ T5020] WARNING: CPU: 65 PID: 5020 at fs/namei.c:2182 link_path_walk+0x29c/0x510
[ 245.766242][ T5020] Modules linked in: kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci mdio libata tg3 libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
[ 245.766289][ T5020] CPU: 65 PID: 5020 Comm: vdo Not tainted 5.6.0-rc7-next-20200324+ #6
[ 245.766314][ T5020] NIP: c0000000004dbffc LR: c0000000004dc0e0 CTR: 0000000000000000
[ 245.766338][ T5020] REGS: c000200ec1fcf6c0 TRAP: 0700 Not tainted (5.6.0-rc7-next-20200324+)
[ 245.766374][ T5020] MSR: 9000000000029033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 28228422 XER: 20040000
[ 245.766415][ T5020] CFAR: c0000000004dc0e4 IRQMASK: 0
[ 245.766415][ T5020] GPR00: c0000000004dc0e0 c000200ec1fcf950 c00000000165a500 0000000000000000
[ 245.766415][ T5020] GPR04: c000000001511408 0000000000000000 c000200ec1fcf6a4 0000000000000002
[ 245.766415][ T5020] GPR08: 0000000000000001 0000000000000000 0000000000000001 0000000000000001
[ 245.766415][ T5020] GPR12: 0000000000008000 c000201fff7fa700 00007fffa6f0f360 c000200579ec7ae0
[ 245.766415][ T5020] GPR16: 7fffffffffffffff fffffffffffffe00 0000000000000000 0000000000000000
[ 245.766415][ T5020] GPR20: c000000c415a2498 c0002011e2cabd83 2f2f2f2f2f2f2f2f 0000000000000003
[ 245.766415][ T5020] GPR24: 0000000000000000 c000200ec1fcfab8 fffffffffffff000 0000000000200000
[ 245.766415][ T5020] GPR28: ffffffffffffffff 61c8864680b583eb 0000000000000001 0000000000002e2e
[ 245.766632][ T5020] NIP [c0000000004dbffc] link_path_walk+0x29c/0x510
[ 245.766655][ T5020] LR [c0000000004dc0e0] link_path_walk+0x380/0x510
[ 245.766677][ T5020] Call Trace:
[ 245.766698][ T5020] [c000200ec1fcf950] [c0000000004dc0e0] link_path_walk+0x380/0x510 (unreliable)
[ 245.766725][ T5020] [c000200ec1fcfa50] [c0000000004dc3c4] path_lookupat+0x94/0x1b0
[ 245.766739][ T5020] [c000200ec1fcfa90] [c0000000004de010] filename_lookup.part.55+0xa0/0x170
[ 245.766778][ T5020] [c000200ec1fcfbd0] [c00000000096cbe8] unix_find_other+0x58/0x3c0
[ 245.766803][ T5020] [c000200ec1fcfc60] [c00000000096de54] unix_stream_connect+0x144/0x870
[ 245.766842][ T5020] [c000200ec1fcfd30] [c0000000007cf860] __sys_connect+0x140/0x170
[ 245.766881][ T5020] [c000200ec1fcfe00] [c0000000007cf8b8] sys_connect+0x28/0x40
[ 245.766897][ T5020] [c000200ec1fcfe20] [c00000000000b378] system_call+0x5c/0x68
[ 245.766921][ T5020] Instruction dump:
[ 245.766952][ T5020] 38800000 7f23cb78 7fde07b4 1d5e0030 7d295214 eaa90020 4bfffab5 2fa30000
[ 245.767003][ T5020] 409e00fc e9390008 7d2a0074 794ad182 <0b0a0000> 2fa90000 419e0184 81290000
[ 245.767031][ T5020] irq event stamp: 27044
[ 245.767052][ T5020] hardirqs last enabled at (27043): [<c0000000004db354>] handle_dots.part.47+0x224/0x960
[ 245.767068][ T5020] hardirqs last disabled at (27044): [<c000000000008fbc>] program_check_common+0x21c/0x230
[ 245.767096][ T5020] softirqs last enabled at (27018): [<c000000000968620>] unix_create1+0x1e0/0x2a0
[ 245.767144][ T5020] softirqs last disabled at (27016): [<c000000000968620>] unix_create1+0x1e0/0x2a0
[ 245.767180][ T5020] ---[ end trace 7b3382b3d92e587f ]---
[ 245.767202][ T5020] pathname = /var/run/nscd/socket
[ 245.767225][ T5020] BUG: Kernel NULL pointer dereference on read at 0x00000000
[ 245.767248][ T5020] Faulting instruction address: 0xc0000000004dc008
[ 245.767298][ T5020] Oops: Kernel access of bad area, sig: 11 [#1]
[ 245.767320][ T5020] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
[ 245.767354][ T5020] Modules linked in: kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci mdio libata tg3 libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
[ 245.767425][ T5020] CPU: 65 PID: 5020 Comm: vdo Tainted: G W 5.6.0-rc7-next-20200324+ #6
[ 245.767460][ T5020] NIP: c0000000004dc008 LR: c0000000004dc19c CTR: 0000000000000000
[ 245.767494][ T5020] REGS: c000200ec1fcf6c0 TRAP: 0300 Tainted: G W (5.6.0-rc7-next-20200324+)
[ 245.767541][ T5020] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 48228222 XER: 20040000
[ 245.767579][ T5020] CFAR: c0000000004dc1a4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
[ 245.767579][ T5020] GPR00: c0000000004dc19c c000200ec1fcf950 c00000000165a500 000000000000001f
[ 245.767579][ T5020] GPR04: 0000000000000006 0000000000000000 c000200ec1fcf6b4 0000000000000007
[ 245.767579][ T5020] GPR08: 0000000000000003 0000000000000000 c000200ec118aa80 0000000000000000
[ 245.767579][ T5020] GPR12: 0000000000008000 c000201fff7fa700 00007fffa6f0f360 c000200579ec7ae0
[ 245.767579][ T5020] GPR16: 7fffffffffffffff fffffffffffffe00 0000000000000000 0000000000000000
[ 245.767579][ T5020] GPR20: c000000c415a2498 c0002011e2cabd83 2f2f2f2f2f2f2f2f 0000000000000003
[ 245.767579][ T5020] GPR24: 0000000000000000 c000200ec1fcfab8 fffffffffffff000 0000000000200000
[ 245.767579][ T5020] GPR28: ffffffffffffffff 61c8864680b583eb 0000000000000001 0000000000002e2e
[ 245.767830][ T5020] NIP [c0000000004dc008] link_path_walk+0x2a8/0x510
[ 245.767852][ T5020] LR [c0000000004dc19c] link_path_walk+0x43c/0x510
[ 245.767884][ T5020] Call Trace:
[ 245.767903][ T5020] [c000200ec1fcf950] [c0000000004dc19c] link_path_walk+0x43c/0x510 (unreliable)
[ 245.767950][ T5020] [c000200ec1fcfa50] [c0000000004dc3c4] path_lookupat+0x94/0x1b0
[ 245.767974][ T5020] [c000200ec1fcfa90] [c0000000004de010] filename_lookup.part.55+0xa0/0x170
[ 245.768070][ T5020] [c000200ec1fcfbd0] [c00000000096cbe8] unix_find_other+0x58/0x3c0
[ 245.768155][ T5020] [c000200ec1fcfc60] [c00000000096de54] unix_stream_connect+0x144/0x870
[ 245.768255][ T5020] [c000200ec1fcfd30] [c0000000007cf860] __sys_connect+0x140/0x170
[ 245.768358][ T5020] [c000200ec1fcfe00] [c0000000007cf8b8] sys_connect+0x28/0x40
[ 245.768468][ T5020] [c000200ec1fcfe20] [c00000000000b378] system_call+0x5c/0x68
[ 245.768545][ T5020] Instruction dump:
[ 245.768609][ T5020] 1d5e0030 7d295214 eaa90020 4bfffab5 2fa30000 409e00fc e9390008 7d2a0074
[ 245.768705][ T5020] 794ad182 0b0a0000 2fa90000 419e0184 <81290000> 55290256 7f89d800 419efe8c
[ 245.768816][ T5020] ---[ end trace 7b3382b3d92e5880 ]---
[ 245.820494][ T5040] sda2: Can't mount, would change RO state
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 3:24 ` Qian Cai
@ 2020-03-25 4:03 ` Al Viro
2020-03-25 5:58 ` Al Viro
2020-03-25 13:21 ` Qian Cai
0 siblings, 2 replies; 12+ messages in thread
From: Al Viro @ 2020-03-25 4:03 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Tue, Mar 24, 2020 at 11:24:01PM -0400, Qian Cai wrote:
> > On Mar 24, 2020, at 10:13 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Tue, Mar 24, 2020 at 09:49:48PM -0400, Qian Cai wrote:
> >
> >> It does not catch anything at all with the patch,
> >
> > You mean, oops happens, but neither WARN_ON() is triggered?
> > Lovely... Just to make sure: could you slap the same couple
> > of lines just before
> > if (unlikely(!d_can_lookup(nd->path.dentry))) {
> > in link_path_walk(), just to check if I have misread the trace
> > you've got?
> >
> > Does that (+ other two inserts) end up with
> > 1) some of these WARN_ON() triggered when oops happens or
> > 2) oops is happening, but neither WARN_ON() triggers or
> > 3) oops not happening / becoming harder to hit?
>
> Only the one just before
> if (unlikely(!d_can_lookup(nd->path.dentry))) {
> In link_path_walk() will trigger.
> [ 245.767202][ T5020] pathname = /var/run/nscd/socket
Lovely. So
* we really do get NULL nd->path.dentry there; I've not misread the
trace.
* on the entry into link_path_walk() nd->path.dentry is non-NULL.
* *ALL* components should've been LAST_NORM ones
* not a single symlink in sight, unless the setup is rather unusual
* possibly not even a single mountpoint along the way (depending
upon the userland used)
And in the same loop we have
if (likely(type == LAST_NORM)) {
struct dentry *parent = nd->path.dentry;
nd->flags &= ~LOOKUP_JUMPED;
if (unlikely(parent->d_flags & DCACHE_OP_HASH)) {
struct qstr this = { { .hash_len = hash_len }, .name = name };
err = parent->d_op->d_hash(parent, &this);
if (err < 0)
return err;
hash_len = this.hash_len;
name = this.name;
}
}
upstream of that thing. So NULL nd->path.dentry *there* would've oopsed.
IOW, what we are hitting is walk_component() with non-NULL nd->path.dentry
when we enter it, NULL being returned and nd->path.dentry becoming NULL
by the time we return from walk_component().
Could you post the results of
stat / /var /var/run /var/run/nscd /var/run/nscd/socket
after the boot with working kernel? Also, is that "hit on every boot" or
stochastic? If it's the latter, I'd like to see the output of the same
thing on a successful boot of the same kernel, if possible...
Also, is the pathname always the same and if not, what other variants have
been observed?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 4:03 ` Al Viro
@ 2020-03-25 5:58 ` Al Viro
2020-03-25 14:02 ` Al Viro
2020-03-25 19:43 ` Qian Cai
2020-03-25 13:21 ` Qian Cai
1 sibling, 2 replies; 12+ messages in thread
From: Al Viro @ 2020-03-25 5:58 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Wed, Mar 25, 2020 at 04:03:59AM +0000, Al Viro wrote:
> Lovely. So
> * we really do get NULL nd->path.dentry there; I've not misread the
> trace.
> * on the entry into link_path_walk() nd->path.dentry is non-NULL.
> * *ALL* components should've been LAST_NORM ones
> * not a single symlink in sight, unless the setup is rather unusual
> * possibly not even a single mountpoint along the way (depending
> upon the userland used)
OK, I see one place where that could occur, but I really don't see how that
could be triggered on this pathname, short of very odd symlink layout in
the filesystem on the testbox. Does the following fix your reproducer?
diff --git a/fs/namei.c b/fs/namei.c
index 311e33dbac63..4082b70f32ff 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1805,6 +1805,8 @@ static const char *handle_dots(struct nameidata *nd, int type)
error = step_into(nd, WALK_NOFOLLOW,
parent, inode, seq);
}
+ if (unlikely(error))
+ return ERR_PTR(error);
if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
/*
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 4:03 ` Al Viro
2020-03-25 5:58 ` Al Viro
@ 2020-03-25 13:21 ` Qian Cai
1 sibling, 0 replies; 12+ messages in thread
From: Qian Cai @ 2020-03-25 13:21 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, linux-fsdevel, LKML
> On Mar 25, 2020, at 12:03 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Mar 24, 2020 at 11:24:01PM -0400, Qian Cai wrote:
>
>>> On Mar 24, 2020, at 10:13 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>>>
>>> On Tue, Mar 24, 2020 at 09:49:48PM -0400, Qian Cai wrote:
>>>
>>>> It does not catch anything at all with the patch,
>>>
>>> You mean, oops happens, but neither WARN_ON() is triggered?
>>> Lovely... Just to make sure: could you slap the same couple
>>> of lines just before
>>> if (unlikely(!d_can_lookup(nd->path.dentry))) {
>>> in link_path_walk(), just to check if I have misread the trace
>>> you've got?
>>>
>>> Does that (+ other two inserts) end up with
>>> 1) some of these WARN_ON() triggered when oops happens or
>>> 2) oops is happening, but neither WARN_ON() triggers or
>>> 3) oops not happening / becoming harder to hit?
>>
>> Only the one just before
>> if (unlikely(!d_can_lookup(nd->path.dentry))) {
>> In link_path_walk() will trigger.
>
>> [ 245.767202][ T5020] pathname = /var/run/nscd/socket
>
> Lovely. So
> * we really do get NULL nd->path.dentry there; I've not misread the
> trace.
> * on the entry into link_path_walk() nd->path.dentry is non-NULL.
> * *ALL* components should've been LAST_NORM ones
> * not a single symlink in sight, unless the setup is rather unusual
> * possibly not even a single mountpoint along the way (depending
> upon the userland used)
>
> And in the same loop we have
> if (likely(type == LAST_NORM)) {
> struct dentry *parent = nd->path.dentry;
> nd->flags &= ~LOOKUP_JUMPED;
> if (unlikely(parent->d_flags & DCACHE_OP_HASH)) {
> struct qstr this = { { .hash_len = hash_len }, .name = name };
> err = parent->d_op->d_hash(parent, &this);
> if (err < 0)
> return err;
> hash_len = this.hash_len;
> name = this.name;
> }
> }
> upstream of that thing. So NULL nd->path.dentry *there* would've oopsed.
> IOW, what we are hitting is walk_component() with non-NULL nd->path.dentry
> when we enter it, NULL being returned and nd->path.dentry becoming NULL
> by the time we return from walk_component().
>
> Could you post the results of
> stat / /var /var/run /var/run/nscd /var/run/nscd/socket
The file is gone after a successful boot,
# stat / /var /var/run /var/run/nscd /var/run/nscd/socket
File: /
Size: 244 Blocks: 0 IO Block: 65536 directory
Device: fe00h/65024d Inode: 128 Links: 17
Access: (0555/dr-xr-xr-x) Uid: ( 0/ root) Gid: ( 0/ root)
Access: 2020-03-24 14:21:27.112559236 -0400
Modify: 2020-03-24 14:21:25.840486593 -0400
Change: 2020-03-24 14:21:25.840486593 -0400
Birth: -
File: /var
Size: 4096 Blocks: 8 IO Block: 65536 directory
Device: fe00h/65024d Inode: 133 Links: 21
Access: (0755/drwxr-xr-x) Uid: ( 0/ root) Gid: ( 0/ root)
Access: 2018-08-12 05:57:57.000000000 -0400
Modify: 2020-03-23 21:29:31.087264900 -0400
Change: 2020-03-23 21:29:31.087264900 -0400
Birth: -
File: /var/run -> ../run
Size: 6 Blocks: 0 IO Block: 65536 symbolic link
Device: fe00h/65024d Inode: 143 Links: 1
Access: (0777/lrwxrwxrwx) Uid: ( 0/ root) Gid: ( 0/ root)
Access: 2020-03-24 17:34:11.865030724 -0400
Modify: 2020-03-23 17:16:40.573974805 -0400
Change: 2020-03-23 17:16:40.573974805 -0400
Birth: -
stat: cannot stat '/var/run/nscd': No such file or directory
stat: cannot stat '/var/run/nscd/socket': No such file or directory
> after the boot with working kernel? Also, is that "hit on every boot" or
> stochastic? If it's the latter, I'd like to see the output of the same
> thing on a successful boot of the same kernel, if possible...
It does not hit every time, so I used a cron job,
@reboot sleep 180; systemctl reboot
It has always hit it within a hour so far.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 5:58 ` Al Viro
@ 2020-03-25 14:02 ` Al Viro
2020-03-25 14:05 ` Al Viro
2020-03-25 19:43 ` Qian Cai
1 sibling, 1 reply; 12+ messages in thread
From: Al Viro @ 2020-03-25 14:02 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Wed, Mar 25, 2020 at 05:58:30AM +0000, Al Viro wrote:
> On Wed, Mar 25, 2020 at 04:03:59AM +0000, Al Viro wrote:
>
> > Lovely. So
> > * we really do get NULL nd->path.dentry there; I've not misread the
> > trace.
> > * on the entry into link_path_walk() nd->path.dentry is non-NULL.
> > * *ALL* components should've been LAST_NORM ones
> > * not a single symlink in sight, unless the setup is rather unusual
> > * possibly not even a single mountpoint along the way (depending
> > upon the userland used)
>
> OK, I see one place where that could occur, but I really don't see how that
> could be triggered on this pathname, short of very odd symlink layout in
> the filesystem on the testbox.
... which, apparently, is what you've got there (/var/run -> ../run), so
stepping into that braino is not implausible. Could you check if the
fix below fixes what you've observed? I am folding it in anyway (into
"lift all calls of step_into() out of follow_dotdot/follow_dotdot_rcu") -
it's an obvious braino introduced in the commit in question, but I'd
like a confirmation that this _is_ what you've caught.
> Does the following fix your reproducer?
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 311e33dbac63..4082b70f32ff 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1805,6 +1805,8 @@ static const char *handle_dots(struct nameidata *nd, int type)
> error = step_into(nd, WALK_NOFOLLOW,
> parent, inode, seq);
> }
> + if (unlikely(error))
> + return ERR_PTR(error);
>
> if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> /*
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 14:02 ` Al Viro
@ 2020-03-25 14:05 ` Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2020-03-25 14:05 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Wed, Mar 25, 2020 at 02:02:07PM +0000, Al Viro wrote:
> > + if (unlikely(error))
> > + return ERR_PTR(error);
return error;
that is
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 5:58 ` Al Viro
2020-03-25 14:02 ` Al Viro
@ 2020-03-25 19:43 ` Qian Cai
2020-03-25 21:07 ` Al Viro
1 sibling, 1 reply; 12+ messages in thread
From: Qian Cai @ 2020-03-25 19:43 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, linux-fsdevel, LKML
> On Mar 25, 2020, at 1:58 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Wed, Mar 25, 2020 at 04:03:59AM +0000, Al Viro wrote:
>
>> Lovely. So
>> * we really do get NULL nd->path.dentry there; I've not misread the
>> trace.
>> * on the entry into link_path_walk() nd->path.dentry is non-NULL.
>> * *ALL* components should've been LAST_NORM ones
>> * not a single symlink in sight, unless the setup is rather unusual
>> * possibly not even a single mountpoint along the way (depending
>> upon the userland used)
>
> OK, I see one place where that could occur, but I really don't see how that
> could be triggered on this pathname, short of very odd symlink layout in
> the filesystem on the testbox. Does the following fix your reproducer?
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 311e33dbac63..4082b70f32ff 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1805,6 +1805,8 @@ static const char *handle_dots(struct nameidata *nd, int type)
> error = step_into(nd, WALK_NOFOLLOW,
> parent, inode, seq);
> }
> + if (unlikely(error))
> + return ERR_PTR(error);
>
> if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> /*
Since that one has a compilation warning, I have tested this patch and seen no crash so far.
diff --git a/fs/namei.c b/fs/namei.c
index 311e33dbac63..73851acdbf3a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1806,6 +1806,9 @@ static const char *handle_dots(struct nameidata *nd, int type)
parent, inode, seq);
}
+ if (unlikely(error))
+ return error;
+
if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
/*
* If there was a racing rename or mount along our
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Null-ptr-deref due to "sanitized pathwalk machinery (v4)"
2020-03-25 19:43 ` Qian Cai
@ 2020-03-25 21:07 ` Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2020-03-25 21:07 UTC (permalink / raw)
To: Qian Cai; +Cc: Linus Torvalds, linux-fsdevel, LKML
On Wed, Mar 25, 2020 at 03:43:06PM -0400, Qian Cai wrote:
> Since that one has a compilation warning, I have tested this patch and seen no crash so far.
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 311e33dbac63..73851acdbf3a 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1806,6 +1806,9 @@ static const char *handle_dots(struct nameidata *nd, int type)
> parent, inode, seq);
> }
>
> + if (unlikely(error))
> + return error;
> +
OK, an equivalent of that had been folded into #work.dotdot/#for-next
I'll definitely throw a mentioning of your reporting that thing;
do you want tested-by: added there as well?
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-03-25 21:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24 21:06 Null-ptr-deref due to "sanitized pathwalk machinery (v4)" Qian Cai
2020-03-24 21:46 ` Al Viro
2020-03-25 1:49 ` Qian Cai
2020-03-25 2:13 ` Al Viro
2020-03-25 3:24 ` Qian Cai
2020-03-25 4:03 ` Al Viro
2020-03-25 5:58 ` Al Viro
2020-03-25 14:02 ` Al Viro
2020-03-25 14:05 ` Al Viro
2020-03-25 19:43 ` Qian Cai
2020-03-25 21:07 ` Al Viro
2020-03-25 13:21 ` Qian Cai
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).