Zhihao Cheng (2): ovl: let helper ovl_i_path_real() return the realinode ovl: fix null pointer dereference in ovl_get_acl_rcu()
fs/overlayfs/inode.c | 12 ++++++------ fs/overlayfs/overlayfs.h | 2 +- fs/overlayfs/util.c | 7 ++++--- 3 files changed, 11 insertions(+), 10 deletions(-)
[ Upstream commit b2dd05f107b11966e26fe52a313b418364cf497b ]
Let helper ovl_i_path_real() return the realinode to prepare for checking non-null realinode in RCU walking path.
[msz] Use d_inode_rcu() since we are depending on the consitency between dentry and inode being non-NULL in an RCU setting.
There are some changes from upstream commit: 1. Context conflicts caused by 73db6a063c785bc ("ovl: port to vfs{g,u}id_t and associated helpers") is handled.
Signed-off-by: Zhihao Cheng chengzhihao1@huawei.com Signed-off-by: Amir Goldstein amir73il@gmail.com Fixes: ffa5723c6d25 ("ovl: store lower path in ovl_inode") Cc: stable@vger.kernel.org # v5.19 Signed-off-by: Miklos Szeredi mszeredi@redhat.com --- fs/overlayfs/overlayfs.h | 2 +- fs/overlayfs/util.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index e74a610a117e..6fb65df09a79 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -367,7 +367,7 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry); void ovl_path_upper(struct dentry *dentry, struct path *path); void ovl_path_lower(struct dentry *dentry, struct path *path); void ovl_path_lowerdata(struct dentry *dentry, struct path *path); -void ovl_i_path_real(struct inode *inode, struct path *path); +struct inode *ovl_i_path_real(struct inode *inode, struct path *path); enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path); enum ovl_path_type ovl_path_realdata(struct dentry *dentry, struct path *path); struct dentry *ovl_dentry_upper(struct dentry *dentry); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 81a57a8d80d9..c9984785999c 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -250,7 +250,7 @@ struct dentry *ovl_i_dentry_upper(struct inode *inode) return ovl_upperdentry_dereference(OVL_I(inode)); }
-void ovl_i_path_real(struct inode *inode, struct path *path) +struct inode *ovl_i_path_real(struct inode *inode, struct path *path) { path->dentry = ovl_i_dentry_upper(inode); if (!path->dentry) { @@ -259,6 +259,8 @@ void ovl_i_path_real(struct inode *inode, struct path *path) } else { path->mnt = ovl_upper_mnt(OVL_FS(inode->i_sb)); } + + return path->dentry ? d_inode_rcu(path->dentry) : NULL; }
struct inode *ovl_inode_upper(struct inode *inode) @@ -1105,8 +1107,7 @@ void ovl_copyattr(struct inode *inode) struct inode *realinode; struct user_namespace *real_mnt_userns;
- ovl_i_path_real(inode, &realpath); - realinode = d_inode(realpath.dentry); + realinode = ovl_i_path_real(inode, &realpath); real_mnt_userns = mnt_user_ns(realpath.mnt);
inode->i_uid = i_uid_into_mnt(real_mnt_userns, realinode);
[ Upstream commit f4e19e595cc2e76a8a58413eb19d3d9c51328b53 ]
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.
There are some changes from upstream commit: 1. Corrusponds to do_ovl_get_acl() in 6.1 is ovl_get_acl() 2. Context conflicts caused by 6c0a8bfb84af8f3 ("ovl: implement get acl method") is handled.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217404 Fixes: 332f606b32b6 ("ovl: enable RCU'd ->get_acl()") Cc: stable@vger.kernel.org # v5.15 Signed-off-by: Zhihao Cheng chengzhihao1@huawei.com Suggested-by: Christian Brauner brauner@kernel.org Suggested-by: Amir Goldstein amir73il@gmail.com Signed-off-by: Amir Goldstein amir73il@gmail.com Signed-off-by: Miklos Szeredi mszeredi@redhat.com --- 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 9e61511de7a7..677649b34965 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -497,20 +497,20 @@ static void ovl_idmap_posix_acl(struct inode *realinode, */ struct posix_acl *ovl_get_acl(struct inode *inode, int type, bool rcu) { - struct inode *realinode = ovl_inode_real(inode); + struct inode *realinode; struct posix_acl *acl, *clone; 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) { acl = get_cached_acl_rcu(realinode, type); } else {
On Mon, Jul 17, 2023 at 11:09:02AM +0800, Zhihao Cheng wrote:
Zhihao Cheng (2): ovl: let helper ovl_i_path_real() return the realinode ovl: fix null pointer dereference in ovl_get_acl_rcu()
fs/overlayfs/inode.c | 12 ++++++------ fs/overlayfs/overlayfs.h | 2 +- fs/overlayfs/util.c | 7 ++++--- 3 files changed, 11 insertions(+), 10 deletions(-)
-- 2.39.2
Now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Zhihao Cheng