[Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Feifei Xu]

[-- Attachment #1: Type: text/plain, Size: 3782 bytes --]

Hi,

I met crashes on ppc64le machine.

Call trace: lookup_fast( ) -> __d_lookup_rcu( ) -> dentry_cmp( ) -> 
dentry_string_cmp ( )
 From the symbolized trace and disassembly code, when doing 
dentry_string_cmp(),
dentry.d_name->name is NULL , this dereference triggered crash.

The dentry's data when crash happens: http://paste.ubuntu.com/19340635/.
And the analysis of the crash vmcore here if you're interested: 
http://paste.ubuntu.com/19359665/

Also pasted above traces on attached txt file.

Can we add check before at the begging of dentry_string_cmp() as below?
Or maybe we should not silently ignore the NULL pointer.
static inline int dentry_string_cmp(const unsigned char *cs, const 
unsigned char *ct, unsigned tcount)
  {
         do {
+             if (unlikely(!cs || !ct ))
+                     return 1;
                 if (*cs != *ct)
                         return 1;
                 cs++;



Below is the stack trace:
--------------------------------------------------------------------------------------------------------- 


Stack trace output:
[387421.142576] Unable to handle kernel paging request for data at 
address 0x00000000
[387421.142709] Faulting instruction address: 0xc000000000327f00
[387421.142769] Oops: Kernel access of bad area, sig: 11 [#1]
[387421.142816] SMP NR_CPUS=2048 NUMA PowerNV
[387421.142876] Modules linked in: iptable_mangle iptable_nat 
nf_nat_ipv4 nf_nat
iptable_raw iptable_filter ip_tables binfmt_misc nf_conntrack_ipv4 
nf_defrag_ipv4
...
[387421.143529] CPU: 69 PID: 39485 Comm: rsync Tainted: G W      
------------   3.10.0-327.18.2.el7.ppc64le #1
[387421.143622] task: c0000022787bd220 ti: c000001f06fc0000 task.ti: 
c000001f06fc0000
[387421.143692] NIP: c000000000327f00 LR: c0000000003122f8 CTR: 
0000000000000008
[387421.143761] REGS: c000001f06fc3820 TRAP: 0300   Tainted: G        
W      ------------    (3.10.0-327.18.2.el7.ppc64le)
[387421.143853] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 
22000882  XER: 00000000
[387421.144026] CFAR: c000000000009368 DAR: 0000000000000000 DSISR: 
40000000 SOFTE: 1
GPR00: c0000000003122f8 c000001f06fc3aa0 c0000000011231b0 c000002611320300
GPR04: c000001f06fc3c60 0000000000000002 0000000000000007 0000000000000000
GPR08: 0000000000000008 ffffffffffffffff c0000029aa14b048 c0000029aa14b049
GPR12: 0000000000000000 c000000007b46d00 0000000000000003 0000000000000018
GPR16: 0000000000000000 00000000001cc131 00000100399fc3b0 0000000000000002
GPR20: 000000004ab52a5c 00003fffe2a2b328 0000000000000001 c000000001179650
GPR24: 0000000000000007 c0000029aa14b049 c000001f06fc3b20 c000001f06fc3c60
GPR28: 00000008d4908d9a c0000026113203c0 c000002611320300 c0000026113203c8
[387421.144948] NIP [c000000000327f00] __d_lookup_rcu+0x150/0x1d0
[387421.145006] LR [c0000000003122f8] lookup_fast+0x68/0x390
[387421.145053] Call Trace:
[387421.145077] [c000001f06fc3aa0] [c00000000031291c] 
link_path_walk+0x2fc/0xba0 (unreliable)
[387421.145159] [c000001f06fc3b00] [c0000000003122f8] 
lookup_fast+0x68/0x390
[387421.145228] [c000001f06fc3b70] [c00000000031352c] 
path_lookupat+0x1bc/0xb60
[387421.145298] [c000001f06fc3c30] [c000000000319440] 
user_path_at_empty+0xc0/0x430
[387421.145380] [c000001f06fc3d30] [c0000000003056f4] 
vfs_fstatat+0x84/0x280
[387421.145449] [c000001f06fc3d90] [c0000000003059c4] 
SyS_newlstat+0x34/0x60
[387421.145520] [c000001f06fc3e30] [c00000000000a17c] system_call+0x38/0xb4
[387421.145589] Instruction dump:
[387421.145651] 39180001 7d0903a6 3959ffff e93f0020 3929ffff 4800001c 
60000000 60000000
[387421.145872] 60000000 60000000 60000000 60420000 <8ce90001> 8d0a0001 
7f874000 409eff4c
[387421.146096] ---[ end trace 7c1c505a25279a32 ]---
[387421.157384]
[387421.157422] Sending IPI to other CPUs
[387421.158535] IPI complete

Thanks
Fiona


[-- Attachment #2: symbolize_dentry_cmp_crash.txt --]
[-- Type: text/plain, Size: 4182 bytes --]

      KERNEL: vmlinux
    DUMPFILE: /home/fedora/vmcore  [PARTIAL DUMP]
        CPUS: 192
	LOAD AVERAGE: 0.05, 0.09, 0.12
       TASKS: 2445
     RELEASE: 3.10.0-327.18.2.el7.ppc64le
     VERSION: #1 SMP Fri Apr 8 05:10:45 EDT 2016
     MACHINE: ppc64le  (3525 Mhz)
      MEMORY: 256 GB
       PANIC: "Unable to handle kernel paging request for data at address 0x00000000"
         PID: 39485
     COMMAND: "rsync"
        TASK: c0000022787bd220  [THREAD_INFO: c000001f06fc0000]
         CPU: 69
       STATE: TASK_RUNNING (PANIC)

--------------------------------------------------------------------------------------------------	   
crash> gdb l*(__d_lookup_rcu+0x150)
0xc000000000327f00 is in __d_lookup_rcu (fs/dcache.c:182).
177     #else
178
179     static inline int dentry_string_cmp(const unsigned char *cs, const unsigned char *ct, unsigned tcount)
180     {
181             do {
182                     if (*cs != *ct)
183                             return 1;
184                     cs++;
185                     ct++;
186                     tcount--;

--------------------------------------------------------------------------------------------------
crash>dis -l __d_lookup_rcu
...
/usr/src/debug/kernel-3.10.0-327.18.2.el7/linux-3.10.0-327.18.2.el7.ppc64le/fs/dcache.c: 212
0xc000000000327edc <__d_lookup_rcu+300>:        ld      r9,32(r31) 
0xc000000000327ee0 <__d_lookup_rcu+304>:        addi    r9,r9,-1
0xc000000000327ee4 <__d_lookup_rcu+308>:        b       0xc000000000327f00 <__d_lookup_rcu+336>
0xc000000000327ee8 <__d_lookup_rcu+312>:        nop
0xc000000000327eec <__d_lookup_rcu+316>:        nop
0xc000000000327ef0 <__d_lookup_rcu+320>:        nop
0xc000000000327ef4 <__d_lookup_rcu+324>:        nop
0xc000000000327ef8 <__d_lookup_rcu+328>:        nop
0xc000000000327efc <__d_lookup_rcu+332>:        ori     r2,r2,0
/usr/src/debug/kernel-3.10.0-327.18.2.el7/linux-3.10.0-327.18.2.el7.ppc64le/fs/dcache.c: 182
0xc000000000327f00 <__d_lookup_rcu+336>:        lbzu    r7,1(r9)  --->Crash Here(0xc000000000327f00)
0xc000000000327f04 <__d_lookup_rcu+340>:        lbzu    r8,1(r10)
0xc000000000327f08 <__d_lookup_rcu+344>:        cmpw    cr7,r7,r8
0xc000000000327f0c <__d_lookup_rcu+348>:        bne     cr7,0xc000000000327e58 <__d_lookup_rcu+168>

In "/usr/src/debug/kernel-3.10.0-327.18.2.el7/linux-3.10.0-327.18.2.el7.ppc64le/fs/dcache.c: 182", convert to assembly code:
0xc000000000327f00 <__d_lookup_rcu+336>:        lbzu    r7,1(r9)  --->Crash here.(read GPR09 (0xffffffffffffffff +1) = 0x0 , It is a bad address.)

--------------------------------------------------------------------------------------------------
r9: c0000026113203c8 is the address of dentry.d_hash, then struct dentry's address is  0xc0000026113203c0

crash> struct dentry 0xc0000026113203c0
struct dentry {
  d_flags = 17301632,
  d_seq = {
    sequence = 2
  },
  d_hash = {
    next = 0xc0000021b8995dc8,
    pprev = 0xc000003618d25288
  },
  d_parent = 0xc000002611320300,
  d_name = {
    {
      {
        hash = 3566243226,
        len = 8
      },
      hash_len = 37925981594
    },
    name = 0x0      ---> name is NULL
  },
  d_inode = 0xc0000018a8e5fdb8,
  d_iname = "features\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000",	     ---> iname is not NULL
  d_lockref = {
    {
      lock_count = 107374182400,
      {
        lock = {
          {
            rlock = {
              raw_lock = {
                slock = 0
              }
            }
          }
        },
        count = 25
      }
    }
  },
  d_op = 0x0,
  d_sb = 0xc000000fa0a2f800,
  d_time = 0,
  d_fsdata = 0x0,
  d_lru = {
    next = 0xc000002611320380,
    prev = 0xc000002611320500
  },
  d_u = {
    d_child = {
      next = 0xc0000026113203a0,
      prev = 0xc0000026113217d0
    },
    d_rcu = {
      next = 0xc0000026113203a0,
      func = 0xc0000026113217d0
    }
  },
  d_subdirs = {
    next = 0xc000002611321710,
    prev = 0xc000002611320510
  },
  d_alias = {
    next = 0x0,
    pprev = 0xc0000018a8e5fed0
  }
}
   

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Al Viro]

On Thu, Jul 14, 2016 at 09:25:41PM +0800, Feifei Xu wrote:
> Hi,
> 
> I met crashes on ppc64le machine.
> 
> Call trace: lookup_fast( ) -> __d_lookup_rcu( ) -> dentry_cmp( ) ->
> dentry_string_cmp ( )
> From the symbolized trace and disassembly code, when doing
> dentry_string_cmp(),
> dentry.d_name->name is NULL , this dereference triggered crash.
> 
> The dentry's data when crash happens: http://paste.ubuntu.com/19340635/.
> And the analysis of the crash vmcore here if you're interested:
> http://paste.ubuntu.com/19359665/
> 
> Can we add check before at the begging of dentry_string_cmp() as below?
> Or maybe we should not silently ignore the NULL pointer.
> static inline int dentry_string_cmp(const unsigned char *cs, const unsigned
> char *ct, unsigned tcount)
>  {
>         do {
> +             if (unlikely(!cs || !ct ))
> +                     return 1;
>                 if (*cs != *ct)
>                         return 1;
>                 cs++;

No, we can not.  Any time you get NULL there is a bug, plain and simple.
Your crash dump shows an impossible state - dentry->d_name.name equal to
NULL.  This should never happen, period.  What's more, you have that
NULL ->d_name.name in crash dump, so it's not a missing barrier somewhere...

> [387421.143529] CPU: 69 PID: 39485 Comm: rsync Tainted: G W
> ------------   3.10.0-327.18.2.el7.ppc64le #1

Wait a sec, what is that doing on l-k?  It's an RH kernel, and nowhere
near the current one, at that...  Anyway,
	a) can you reproduce it with the mainline?
	b) can you reproduce it on more current RH kernel?
	c) how hard it is to reproduce?  You've mentioned "crashes", which
sounds like there had been more than one...

Another question: which filesystem type had that been?  If you have that
crashdump, dentry->d_sb->s_type->name should give that information...

I don't see any likely candidates for that bug - not in mainline, not in
the kernel you'd been running there.  Basically, once we have obtained
a pointer to dentry (which should happen only in __d_alloc()[1]), it should
never have NULL in its ->d_name.name.  Any path through __d_alloc() that
doesn't end up returning NULL will pass through
        dname[name->len] = 0;

        /* Make sure we always see the terminating NUL character */
        smp_wmb();
        dentry->d_name.name = dname;
which obviously can't end up with dentry->d_name.name == NULL - dname would
have to be NULL as well, and that would oops in the first of the quoted lines.

Nothing should be doing wholesale assignments to ->d_name.  Nothing that
had &dentry->d_name passed as an argument should modify its ->name field
(most of such parameters are declared const struct qstr *, and at least in
mainline all of them could be constified that way; I hadn't scanned the
kernel you'd been using yet - it's several hundred calls to RTFS through ;-/)
Nothing outside of fs/dcache.c should modify ->d_name directly either (and
that I'd verified both for mainline and for 3.10.0-327.el7).

And in fs/dcache.c we have very few places modifying that sucker.  Namely,
swap_names() and copy_name() in mainline and switch_name() in 3.10.0-327.el7.
Assigned values are either ->d_name.name of another dentry or ->d_iname of
this dentry.  The latter is never NULL (it's an array embedded into struct
dentry) and the former would have to have become NULL at some earlier point.

Buggered barriers might be a possibility, but those would probably not leak
all the way into crashdump; I rather doubt that they are buggered, anyway,
since we have store non-NULL into ->d_name.name/lwsync/store a mangled address
of dentry into hash chain vs. fetch a value from hash chain, demangle it into
dentry address, load dentry->d_name.name and observe NULL there.  With
another lwsync thrown in between the last two steps, actually, but even
without that the lwsync in writer + address dependency in reader should've
been enough.

[1] struct dentry is never an object with static or automatic storage duration
or a member of any kind of compound object and the only place where a pointer
to any other kind of object is converted into pointer to struct dentry is
dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); in __d_alloc().  IOW,
all instances of struct dentry must come from that function, period.

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[hejianet]

On 7/20/16 1:59 PM, Al Viro wrote:
> On Thu, Jul 14, 2016 at 09:25:41PM +0800, Feifei Xu wrote:
>> Hi,
>>
>> I met crashes on ppc64le machine.
>>
>> Call trace: lookup_fast( ) -> __d_lookup_rcu( ) -> dentry_cmp( ) ->
>> dentry_string_cmp ( )
>>  From the symbolized trace and disassembly code, when doing
>> dentry_string_cmp(),
>> dentry.d_name->name is NULL , this dereference triggered crash.
>>
>> The dentry's data when crash happens: http://paste.ubuntu.com/19340635/.
>> And the analysis of the crash vmcore here if you're interested:
>> http://paste.ubuntu.com/19359665/
>>
>> Can we add check before at the begging of dentry_string_cmp() as below?
>> Or maybe we should not silently ignore the NULL pointer.
>> static inline int dentry_string_cmp(const unsigned char *cs, const unsigned
>> char *ct, unsigned tcount)
>>   {
>>          do {
>> +             if (unlikely(!cs || !ct ))
>> +                     return 1;
>>                  if (*cs != *ct)
>>                          return 1;
>>                  cs++;
> No, we can not.  Any time you get NULL there is a bug, plain and simple.
> Your crash dump shows an impossible state - dentry->d_name.name equal to
> NULL.  This should never happen, period.  What's more, you have that
> NULL ->d_name.name in crash dump, so it's not a missing barrier somewhere...
>
>> [387421.143529] CPU: 69 PID: 39485 Comm: rsync Tainted: G W
>> ------------   3.10.0-327.18.2.el7.ppc64le #1
> Wait a sec, what is that doing on l-k?  It's an RH kernel, and nowhere
> near the current one, at that...  Anyway,
> 	a) can you reproduce it with the mainline?
> 	b) can you reproduce it on more current RH kernel?
> 	c) how hard it is to reproduce?  You've mentioned "crashes", which
> sounds like there had been more than one...
>
> Another question: which filesystem type had that been?  If you have that
> crashdump, dentry->d_sb->s_type->name should give that information...
>
> I don't see any likely candidates for that bug - not in mainline, not in
> the kernel you'd been running there.  Basically, once we have obtained
> a pointer to dentry (which should happen only in __d_alloc()[1]), it should
> never have NULL in its ->d_name.name.  Any path through __d_alloc() that
> doesn't end up returning NULL will pass through
>          dname[name->len] = 0;
>
>          /* Make sure we always see the terminating NUL character */
>          smp_wmb();
>          dentry->d_name.name = dname;
> which obviously can't end up with dentry->d_name.name == NULL - dname would
> have to be NULL as well, and that would oops in the first of the quoted lines.
Maybe not
This barrier is to guarantee that in dentry_cmp,
if dentry->d_name.name is equal to dname (not NULL), then dname[name->len]
should be equal to 0(dname is terminated with NULL).
This barrier will NOT guarantee that dentry->d_name.name != NULL in dentry_cmp.
So maybe we need to add a if statement to avoid this race condition?

Is there anything wrong with my descriptions?
B.R.
Justin
>
> Nothing should be doing wholesale assignments to ->d_name.  Nothing that
> had &dentry->d_name passed as an argument should modify its ->name field
> (most of such parameters are declared const struct qstr *, and at least in
> mainline all of them could be constified that way; I hadn't scanned the
> kernel you'd been using yet - it's several hundred calls to RTFS through ;-/)
> Nothing outside of fs/dcache.c should modify ->d_name directly either (and
> that I'd verified both for mainline and for 3.10.0-327.el7).
>
> And in fs/dcache.c we have very few places modifying that sucker.  Namely,
> swap_names() and copy_name() in mainline and switch_name() in 3.10.0-327.el7.
> Assigned values are either ->d_name.name of another dentry or ->d_iname of
> this dentry.  The latter is never NULL (it's an array embedded into struct
> dentry) and the former would have to have become NULL at some earlier point.
>
> Buggered barriers might be a possibility, but those would probably not leak
> all the way into crashdump; I rather doubt that they are buggered, anyway,
> since we have store non-NULL into ->d_name.name/lwsync/store a mangled address
> of dentry into hash chain vs. fetch a value from hash chain, demangle it into
> dentry address, load dentry->d_name.name and observe NULL there.  With
> another lwsync thrown in between the last two steps, actually, but even
> without that the lwsync in writer + address dependency in reader should've
> been enough.
>
> [1] struct dentry is never an object with static or automatic storage duration
> or a member of any kind of compound object and the only place where a pointer
> to any other kind of object is converted into pointer to struct dentry is
> dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); in __d_alloc().  IOW,
> all instances of struct dentry must come from that function, period.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Al Viro]

On Thu, Jul 21, 2016 at 11:20:16AM +0800, hejianet wrote:

> > I don't see any likely candidates for that bug - not in mainline, not in
> > the kernel you'd been running there.  Basically, once we have obtained
> > a pointer to dentry (which should happen only in __d_alloc()[1]), it should
> > never have NULL in its ->d_name.name.  Any path through __d_alloc() that
> > doesn't end up returning NULL will pass through
> >          dname[name->len] = 0;
> > 
> >          /* Make sure we always see the terminating NUL character */
> >          smp_wmb();
> >          dentry->d_name.name = dname;
> > which obviously can't end up with dentry->d_name.name == NULL - dname would
> > have to be NULL as well, and that would oops in the first of the quoted lines.
> Maybe not
> This barrier is to guarantee that in dentry_cmp,
> if dentry->d_name.name is equal to dname (not NULL), then dname[name->len]
> should be equal to 0(dname is terminated with NULL).
> This barrier will NOT guarantee that dentry->d_name.name != NULL in dentry_cmp.
> So maybe we need to add a if statement to avoid this race condition?
> 
> Is there anything wrong with my descriptions?

	Hash insertion does smp_store_release().  Hash chain traversal -
smp_read_barrier_depends().  On ppc the former is lwsync, while the latter
is no-op, so it boils down to
	store dentry->d_name.name
	lwsync
	store mangled address of dentry into hash chain
vs.
	fetch mangled address of dentry
	demangle it
	fetch dentry->d_name.name
which should be enough - lwsync paired with address dependency gives the
ordering.  IOW, it's not about the barriers in __d_alloc(), it's those in
hlist_bl_add_head_rcu() and hlist_bl_for_each_entry_rcu().

	And it couldn't be a missing barrier anyway - crash dump shows that
sucker with NULL ->d_name.name.

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Al Viro]

On Thu, Jul 21, 2016 at 05:18:57AM +0100, Al Viro wrote:
> 	Hash insertion does smp_store_release().  Hash chain traversal -
> smp_read_barrier_depends().  On ppc the former is lwsync, while the latter
> is no-op, so it boils down to
> 	store dentry->d_name.name
> 	lwsync
> 	store mangled address of dentry into hash chain
> vs.
> 	fetch mangled address of dentry
> 	demangle it
> 	fetch dentry->d_name.name
> which should be enough - lwsync paired with address dependency gives the
> ordering.  IOW, it's not about the barriers in __d_alloc(), it's those in
> hlist_bl_add_head_rcu() and hlist_bl_for_each_entry_rcu().
> 
> 	And it couldn't be a missing barrier anyway - crash dump shows that
> sucker with NULL ->d_name.name.

FWIW, originally I thought it might be a missing barrier; Paul McKenney
had pointed to barriers in RCU lists primitives.  And crashdump is pretty
much conclusive - broken barriers or not, having a store in some thread
*not* seen by crashdump writer is hard to believe...

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Feifei Xu]

On 2016/7/20 13:59, Al Viro wrote:
>> [387421.143529] CPU: 69 PID: 39485 Comm: rsync Tainted: G W
>> ------------   3.10.0-327.18.2.el7.ppc64le #1
> Wait a sec, what is that doing on l-k?  It's an RH kernel, and nowhere
> near the current one, at that...  Anyway,
> 	a) can you reproduce it with the mainline?
> 	b) can you reproduce it on more current RH kernel?
I am building the 4.6 kernel from mainline to try to reproduce it.
And also will try more current RH kernel-3.10.0-327.22.2.el7.
Will update later.
> 	c) how hard it is to reproduce?  You've mentioned "crashes", which
> sounds like there had been more than one...
I have no test case to reproduce it. But it crashes regularly. And it 
happens on 3 physical
machines (powerpc).
> Another question: which filesystem type had that been?  If you have that
> crashdump, dentry->d_sb->s_type->name should give that information...
   It is xfs.   http://paste.ubuntu.com/20278867/
> I don't see any likely candidates for that bug - not in mainline, not in
> the kernel you'd been running there.  Basically, once we have obtained
> a pointer to dentry (which should happen only in __d_alloc()[1]), it should
> never have NULL in its ->d_name.name.  Any path through __d_alloc() that
> doesn't end up returning NULL will pass through
>          dname[name->len] = 0;
>
>          /* Make sure we always see the terminating NUL character */
>          smp_wmb();
>          dentry->d_name.name = dname;
> which obviously can't end up with dentry->d_name.name == NULL - dname would
> have to be NULL as well, and that would oops in the first of the quoted lines.
>
> Nothing should be doing wholesale assignments to ->d_name.  Nothing that
> had &dentry->d_name passed as an argument should modify its ->name field
> (most of such parameters are declared const struct qstr *, and at least in
> mainline all of them could be constified that way; I hadn't scanned the
> kernel you'd been using yet - it's several hundred calls to RTFS through ;-/)
> Nothing outside of fs/dcache.c should modify ->d_name directly either (and
> that I'd verified both for mainline and for 3.10.0-327.el7).
>
> And in fs/dcache.c we have very few places modifying that sucker.  Namely,
> swap_names() and copy_name() in mainline and switch_name() in 3.10.0-327.el7.
> Assigned values are either ->d_name.name of another dentry or ->d_iname of
> this dentry.  The latter is never NULL (it's an array embedded into struct
> dentry) and the former would have to have become NULL at some earlier point.
>
> Buggered barriers might be a possibility, but those would probably not leak
> all the way into crashdump; I rather doubt that they are buggered, anyway,
> since we have store non-NULL into ->d_name.name/lwsync/store a mangled address
> of dentry into hash chain vs. fetch a value from hash chain, demangle it into
> dentry address, load dentry->d_name.name and observe NULL there.  With
> another lwsync thrown in between the last two steps, actually, but even
> without that the lwsync in writer + address dependency in reader should've
> been enough.
>
> [1] struct dentry is never an object with static or automatic storage duration
> or a member of any kind of compound object and the only place where a pointer
> to any other kind of object is converted into pointer to struct dentry is
> dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); in __d_alloc().  IOW,
> all instances of struct dentry must come from that function, period.
>
Thanks
Fiona

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Feifei Xu]

On 2016/7/21 12:18, Al Viro wrote:
>
> 	Hash insertion does smp_store_release().  Hash chain traversal -
> smp_read_barrier_depends().  On ppc the former is lwsync, while the latter
> is no-op, so it boils down to
> 	store dentry->d_name.name
> 	lwsync
> 	store mangled address of dentry into hash chain
> vs.
> 	fetch mangled address of dentry
> 	demangle it
> 	fetch dentry->d_name.name
> which should be enough - lwsync paired with address dependency gives the
> ordering.  IOW, it's not about the barriers in __d_alloc(), it's those in
> hlist_bl_add_head_rcu() and hlist_bl_for_each_entry_rcu().
>
> 	And it couldn't be a missing barrier anyway - crash dump shows that
> sucker with NULL ->d_name.name.
>
Maybe this is useful: there's a warning that in dmesg before the oops: 
http://paste.ubuntu.com/20279712/

[379630.827833] ------------[ cut here ]------------
[379630.827834] WARNING: at lib/list_debug.c:59
[379630.827835] Modules linked in: iptable_mangle iptable_nat nf_nat_ipv4 nf_nat iptable_raw iptable_filter ip_tables binfmt_misc nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack xfs libcrc32c ses enclosure sg ipmi_powernv ipmi_msghandler rtc_opal i2c_opal i2c_core powernv_rng shpchp ext4 mbcache jbd2 dm_service_time sd_mod sr_mod crc_t10dif crct10dif_generic cdrom crct10dif_common ipr tg3 cxgb4 libata ptp pps_core dm_mirror dm_region_hash dm_log dm_multipath nf_conntrack_ftp dm_mod nf_conntrack [last unloaded: ip_tables]
[379630.827876] CPU: 120 PID: 1371 Comm: kswapd17 Not tainted 3.10.0-327.18.2.el7.ppc64le #1
[379630.827877] task: c000000fe4c49c80 ti: c000000fe4ca0000 task.ti: c000000fe4ca0000
[379630.827879] NIP: c0000000004c9964 LR: c0000000004c9960 CTR: c0000000004b2860
[379630.827880] REGS: c000000fe4ca36a0 TRAP: 0700   Not tainted  (3.10.0-327.18.2.el7.ppc64le)
[379630.827881] MSR: 9000000100029033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 28044044  XER: 20000000
[379630.827886] CFAR: c000000000956fb8 SOFTE: 1
GPR00: c0000000004c9960 c000000fe4ca3920 c0000000011231b0 0000000000000054
GPR04: c00000010ce08018 c00000010ce18bf0 0000000000000000 0000000000000002
GPR08: c000000000cb31b0 0000000000000000 0000000000000000 3239343331313632
GPR12: 0000000042044042 c000000007b63800 0000000000000000 0000000000000d63
GPR16: c000000001055848 00000000000001d4 00000000000f0b03 c000000001055868
GPR20: c000000fa0a2fbd8 00000000000f0b02 0000000000000040 0000000000000078
GPR24: 0000000000000000 c000000fe4ca39b0 c000000fe4ca39a0 0000000000000000
GPR28: c000000001162580 c000000fa0a2f8d0 c000000fa0a2f800 c000002611349200
[379630.827904] NIP [c0000000004c9964] __list_del_entry+0xb4/0xe0
[379630.827906] LR [c0000000004c9960] __list_del_entry+0xb0/0xe0
[379630.827907] Call Trace:
[379630.827909] [c000000fe4ca3920] [c0000000004c9960] __list_del_entry+0xb0/0xe0 (unreliable)
[379630.827913] [c000000fe4ca3980] [c0000000003270dc] prune_dcache_sb+0x9c/0x250
[379630.827915] [c000000fe4ca3a10] [c0000000002ff558] prune_super+0x1b8/0x200
[379630.827918] [c000000fe4ca3a50] [c00000000025adf4] shrink_slab+0x204/0x3b0
[379630.827920] [c000000fe4ca3b50] [c00000000025f00c] balance_pgdat+0x8ac/0xb20
[379630.827922] [c000000fe4ca3c90] [c00000000025f444] kswapd+0x1c4/0x740
[379630.827924] [c000000fe4ca3d80] [c00000000011023c] kthread+0xec/0x100
[379630.827927] [c000000fe4ca3e30] [c00000000000a474] ret_from_kernel_thread+0x5c/0x68
[379630.827928] Instruction dump:
[379630.827929] 0fe00000 4bffffd8 3c62ffa5 38639668 4848d5f9 60000000 0fe00000 4bffffc0
[379630.827931] 3c62ffa5 38639628 4848d5e1 60000000 <0fe00000> 4bffffa8 3c62ffa5 7d254b78
[379630.827935] ---[ end trace 7c1c505a25279a31 ]---

Thanks
Fiona

Re: [Bug] fs/dcache.c: NULL pointer dereference on dentry_string_cmp

[Feifei Xu]

On 2016/7/21 14:40, Feifei Xu wrote:
>
> Maybe this is useful: there's a warning that in dmesg before the oops: 
> http://paste.ubuntu.com/20279712/
>
I missed a line on the warning :

[379630.827815] list_del corruption. prev->next should be c000002611349200, but was           (null)

> [379630.827833] ------------[ cut here ]------------
> [379630.827834] WARNING: at lib/list_debug.c:59
Updated the http://paste.ubuntu.com/20283335/

Thanks
Fiona