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()")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Suggested-by: Christian Brauner <brauner@kernel.org>
---
fs/overlayfs/inode.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index cc3ef5a6666a..b2021eada8be 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -576,20 +576,18 @@ 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;
+ int err;
+
+ err = ovl_i_path_realinode(inode, &realpath, &realinode, rcu);
+ if (err)
+ return ERR_PTR(err);
if (!IS_POSIXACL(realinode))
return NULL;
- /* Careful in RCU walk mode */
- ovl_i_path_real(inode, &realpath);
- if (!realpath.dentry) {
- WARN_ON(!rcu);
- return ERR_PTR(-ECHILD);
- }
-
if (rcu) {
/*
* If the layer is idmapped drop out of RCU path walk
--
2.39.2On Mon, May 15, 2023 at 4:39 PM Zhihao Cheng <chengzhihao1@huawei.com> 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.
AFAICT, both your patches should apply cleanly to 6.1.y, so should
be picked up automatically by stable kernel bots.
Thanks,
Amir.
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Suggested-by: Christian Brauner <brauner@kernel.org>
> ---
> fs/overlayfs/inode.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index cc3ef5a6666a..b2021eada8be 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -576,20 +576,18 @@ 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;
> + int err;
> +
> + err = ovl_i_path_realinode(inode, &realpath, &realinode, rcu);
> + if (err)
> + return ERR_PTR(err);
>
> if (!IS_POSIXACL(realinode))
> return NULL;
>
> - /* Careful in RCU walk mode */
> - ovl_i_path_real(inode, &realpath);
> - if (!realpath.dentry) {
> - WARN_ON(!rcu);
> - return ERR_PTR(-ECHILD);
> - }
> -
> if (rcu) {
> /*
> * If the layer is idmapped drop out of RCU path walk
> --
> 2.39.2
>
在 2023/5/16 19:40, Amir Goldstein 写道:
> On Mon, May 15, 2023 at 4:39 PM Zhihao Cheng <chengzhihao1@huawei.com> 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)?
> AFAICT, both your patches should apply cleanly to 6.1.y, so should
> be picked up automatically by stable kernel bots.
>
> Thanks,
> Amir.
>
>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>> Suggested-by: Christian Brauner <brauner@kernel.org>
>> ---
>> fs/overlayfs/inode.c | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
>> index cc3ef5a6666a..b2021eada8be 100644
>> --- a/fs/overlayfs/inode.c
>> +++ b/fs/overlayfs/inode.c
>> @@ -576,20 +576,18 @@ 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;
>> + int err;
>> +
>> + err = ovl_i_path_realinode(inode, &realpath, &realinode, rcu);
>> + if (err)
>> + return ERR_PTR(err);
>>
>> if (!IS_POSIXACL(realinode))
>> return NULL;
>>
>> - /* Careful in RCU walk mode */
>> - ovl_i_path_real(inode, &realpath);
>> - if (!realpath.dentry) {
>> - WARN_ON(!rcu);
>> - return ERR_PTR(-ECHILD);
>> - }
>> -
>> if (rcu) {
>> /*
>> * If the layer is idmapped drop out of RCU path walk
>> --
>> 2.39.2
>>
> .
>
On Tue, May 16, 2023 at 3:25 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>
> 在 2023/5/16 19:40, Amir Goldstein 写道:
> > On Mon, May 15, 2023 at 4:39 PM Zhihao Cheng <chengzhihao1@huawei.com> 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.
© 2016 - 2026 Red Hat, Inc.