On Tue, May 16, 2023 at 5:19 PM Zhihao Cheng <chengzhihao1@xxxxxxxxxx> 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]. > > Use the helper ovl_i_path_realinode() to get realinode and then do > non-nullptr checking. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217404 > Fixes: 332f606b32b6 ("ovl: enable RCU'd ->get_acl()") > Signed-off-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx> > Suggested-by: Christian Brauner <brauner@xxxxxxxxxx> > Suggested-by: Amir Goldstein <amir73il@xxxxxxxxx> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx> > --- > fs/overlayfs/inode.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c > index ca56b1328a2c..e7e888dea634 100644 > --- a/fs/overlayfs/inode.c > +++ b/fs/overlayfs/inode.c > @@ -558,20 +558,20 @@ struct posix_acl *do_ovl_get_acl(struct mnt_idmap *idmap, > struct inode *inode, int type, > bool rcu, bool noperm) > { > - struct inode *realinode = ovl_inode_real(inode); > + struct inode *realinode; > struct posix_acl *acl; > struct path realpath; > > - if (!IS_POSIXACL(realinode)) > - return NULL; > - > /* Careful in RCU walk mode */ > - ovl_i_path_real(inode, &realpath); > - if (!realpath.dentry) { > + realinode = ovl_i_path_real(inode, &realpath); > + if (!realinode) { > WARN_ON(!rcu); > return ERR_PTR(-ECHILD); > } > > + if (!IS_POSIXACL(realinode)) > + return NULL; > + > if (rcu) { > /* > * If the layer is idmapped drop out of RCU path walk > -- > 2.39.2 >
