Re: NULL pointer dereference when access /proc/net

[haosdent]

> In the kernels of 4.8..4.18 period there it used to do
> so, but only in non-RCU mode (which is the reason for explicit rcu argument passed
> through that callchain).

Yep, we saw the `inode` parameter pass to `__atime_needs_update` is already NULL

```
bool __atime_needs_update(const struct path *path, struct inode *inode,
  bool rcu)
{
struct vfsmount *mnt = path->mnt;
struct timespec now;

if (inode->i_flags & S_NOATIME)   <=== Oops at here because the params
inode is NULL
return false;
```

```
    [exception RIP: __atime_needs_update+5]
    ...  **RSI: 0000000000000000**  <=== the second params of
__atime_needs_update "struct inode *inode" is NULL
```

On Mon, Apr 26, 2021 at 1:22 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Apr 26, 2021 at 01:04:46AM +0800, haosdent wrote:
> > Hi, Alexander, thanks a lot for your quick reply.
> >
> > > Not really - the crucial part is ->d_count == -128, i.e. it's already past
> > > __dentry_kill().
> >
> > Thanks a lot for your information, we would check this.
> >
> > > Which tree is that?
> > > If you have some patches applied on top of that...
> >
> > We use Ubuntu Linux Kernel "4.15.0-42.45~16.04.1" from launchpad directly
> > without any modification,  the mapping Linux Kernel should be
> > "4.15.18" according
> > to https://people.canonical.com/~kernel/info/kernel-version-map.html
>
> Umm...  OK, I don't have it Ubuntu source at hand, but the thing to look into
> would be
>         * nd->flags contains LOOKUP_RCU
>         * in the mainline from that period (i.e. back when __atime_needs_update()
> used to exist) we had atime_needs_update_rcu() called in get_link() under those
> conditions, with
> static inline bool atime_needs_update_rcu(const struct path *path,
>                                           struct inode *inode)
> {
>         return __atime_needs_update(path, inode, true);
> }
> and __atime_needs_update() passing its last argument (rcu:true in this case) to
> relatime_need_update() in
>         if (!relatime_need_update(path, inode, now, rcu))
> relatime_need_update() hitting
>         update_ovl_inode_times(path->dentry, inode, rcu);
> and update_ovl_inode_times() starting with
>         if (rcu || likely(!(dentry->d_flags & DCACHE_OP_REAL)))
>                 return;
> with subsequent accesses to ->d_inode.  Those obviously are *NOT* supposed
> to be reached in rcu mode, due to that check.
>
> Your oops looks like something similar to that call chain had been involved and
> somehow had managed to get through to those ->d_inode uses.
>
> Again, in RCU mode we really, really should not assume ->d_inode stable.  That's
> why atime_needs_update() gets inode as a separate argument and does *NOT* look
> at path->dentry at all.  In the kernels of 4.8..4.18 period there it used to do
> so, but only in non-RCU mode (which is the reason for explicit rcu argument passed
> through that callchain).



-- 
Best Regards,
Haosdent Huang