2023-05-16 13:01:47

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ovl: get_acl: Fix null pointer dereference at realinode in rcu-walk mode

On Tue, May 16, 2023 at 3:25 PM Zhihao Cheng <[email protected]> wrote:
>
> 在 2023/5/16 19:40, Amir Goldstein 写道:
> > On Mon, May 15, 2023 at 4:39 PM Zhihao Cheng <[email protected]> wrote:
> >>
> >> Following process:
> >> P1 P2
> >> path_openat
> >> link_path_walk
> >> may_lookup
> >> inode_permission(rcu)
> >> ovl_permission
> >> acl_permission_check
> >> check_acl
> >> get_cached_acl_rcu
> >> ovl_get_inode_acl
> >> realinode = ovl_inode_real(ovl_inode)
> >> drop_cache
> >> __dentry_kill(ovl_dentry)
> >> iput(ovl_inode)
> >> ovl_destroy_inode(ovl_inode)
> >> dput(oi->__upperdentry)
> >> dentry_kill(upperdentry)
> >> dentry_unlink_inode
> >> upperdentry->d_inode = NULL
> >> ovl_inode_upper
> >> upperdentry = ovl_i_dentry_upper(ovl_inode)
> >> d_inode(upperdentry) // returns NULL
> >> IS_POSIXACL(realinode) // NULL pointer dereference
> >> , will trigger an null pointer dereference at realinode:
> >> [ 205.472797] BUG: kernel NULL pointer dereference, address:
> >> 0000000000000028
> >> [ 205.476701] CPU: 2 PID: 2713 Comm: ls Not tainted
> >> 6.3.0-12064-g2edfa098e750-dirty #1216
> >> [ 205.478754] RIP: 0010:do_ovl_get_acl+0x5d/0x300
> >> [ 205.489584] Call Trace:
> >> [ 205.489812] <TASK>
> >> [ 205.490014] ovl_get_inode_acl+0x26/0x30
> >> [ 205.490466] get_cached_acl_rcu+0x61/0xa0
> >> [ 205.490908] generic_permission+0x1bf/0x4e0
> >> [ 205.491447] ovl_permission+0x79/0x1b0
> >> [ 205.491917] inode_permission+0x15e/0x2c0
> >> [ 205.492425] link_path_walk+0x115/0x550
> >> [ 205.493311] path_lookupat.isra.0+0xb2/0x200
> >> [ 205.493803] filename_lookup+0xda/0x240
> >> [ 205.495747] vfs_fstatat+0x7b/0xb0
> >>
> >> Fetch a reproducer in [Link].
> >>
> >> Fix it by using helper ovl_i_path_realinode() to get realpath and real
> >> inode after non-nullptr checking.
> >>
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217404
> >> Fixes: 332f606b32b6 ("ovl: enable RCU'd ->get_acl()")
> >
> > Note that this bug is also in 5.15.y, in method ovl_get_acl().
> > I hope you will be able to follow up with a simple backport for 5.15 -
> > i.e. only need to add a check for NULL realinode at the beginning.
> > There was no realpath back then.
> >
>
> Sure. I notice that there is a '[ Upstream commit xxx ]' field in 5.15
> patch, so may I backport it after the fix applied into mainline(6.4)?
>

Yes, you need to wait until the fix is merged to mainline before
posting a backport.

You'd usually prefix the patch subject with [PATCH 5.15] and send
it to stable@kernel.

It'd be good to mention the changes from upstream commit,
because this is not a trivial backport.

Thanks,
Amir.