From: Andrey Kalachev Date: Fri, 19 Apr 2024 20:55:10 +0300 Subject: [PATCH] ovl: fix general protection fault in security_inode_getattr general protection fault caused by dentry shared between two processes. Suppose we have a regular file `A` onto lower overlayfs layer, metacopy=on. Process P1 execute link syscall ( `A` link to `B`), P2 do open `B`. P1 | P2 -----------+-------------- sys_link | | sys_open | ovl_lookup B (1) | ovl_alloc_entry (2) | ovl_link | (3) | | sys_link | (4) | | ovl_open B (5) | ovl_copy_up_one | ovl_path_lower | ovl_numlower(oe) (6) | ovl_path_lower (7) | vfs_getattr(struct path, ..) | security_inode_getattr(struct path, ...) | d_backing_inode(path->dentry) (1) P2 call ovl_lookup B, lookup non existent file `B`, new `B` dentry allocated. (2) P2 call ovl_alloc_entry() in `non existent file` context, return zero filled ovl_entry. (3) P1 make hardlink A -> B, now `B` dentry associated with lower layer file `A`. (4) P1 return to userspace from `sys_link`, leave ovl_entry `B` unchanged (zeros). (5) P2 continue open file behind dentry `B`. (6) P2 ovl_numlower(oe) return 0, taken from zero filled ovl_entry `oe` (7) P2 ovl_path_lower() return zero filled `struct path`, due numlower=0. (8) P2 NULL dereference in d_backing_inode() Since release v6.5 `ovl_entry` associated with inode and placed into the `struct ovl_inode_params`. Issue fixed at: commit 0af950f57fef ("ovl: move ovl_entry into ovl_inode") Solution, proposed by Hillf Danton, most common and suitable for stable kernels before v6.5. The patch code is based on it. Reported-by: syzbot+f07cc9be8d1d226947ed@syzkaller.appspotmail.com Link: https://lore.kernel.org/lkml/0000000000008caae305ab9a5318@google.com Link: https://syzkaller.appspot.com/bug?extid=f07cc9be8d1d226947ed Signed-off-by: Andrey Kalachev --- fs/overlayfs/copy_up.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 65ac504595ba..b7352c43b673 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -879,6 +879,13 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, return -EROFS; ovl_path_lower(dentry, &ctx.lowerpath); + + if (unlikely(!ctx.lowerpath.dentry)) { + /* syzbot+f07cc9be8d1d226947ed@syzkaller.appspotmail.com */ + pr_err("prevention GPF in security_inode_getattr()\n"); + return -EIO; + } + err = vfs_getattr(&ctx.lowerpath, &ctx.stat, STATX_BASIC_STATS, AT_STATX_SYNC_AS_STAT); if (err) -- 2.30.2