* [PATCH] ovl: fix oops in ovl_rename
@ 2021-09-24 1:16 Zheng Liang
2021-09-24 7:12 ` 139
2021-09-24 19:01 ` Miklos Szeredi
0 siblings, 2 replies; 4+ messages in thread
From: Zheng Liang @ 2021-09-24 1:16 UTC (permalink / raw)
To: miklos; +Cc: linux-unionfs, zhengliang6, yi.zhang
We find a kernel NULL pointer dereference problem in overlayfs.
The problem can appear in the following scene:
mkdir lower upper work merge
touch lower/old
touch lower/new
mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work merge
rm merge/new
------------------------------------------------------------------------------------------
process A(rename file in merge) process B(delete file in upper)
renameat2(AT_FDCWD, "old", AT_FDCWD, "new", 0)
(new is whiteout file in upper)
do_renameat2
vfs_rename
ovl_rename
(overwrite=true,ovl_lower_positive(old)=true,
ovl_dentry_is_whiteout(new)=true)
(we can add some delay after "flags|=RENAME_EXCHANGE",
it can make the problem appear more easy)
…… unlink(new)
…… (delete whiteout in upper)
(newdentry is negative)
ovl_do_rename
So,before commencing with ovl_do_rename that the flags maybe attach RENAME_EXCHANGE
and the newdentry is negative in ovl_rename.If we enabled selinux,it
will lead to kernel panic.such as the following log:
PID: 2552045 TASK: ffff8880302faf00 CPU: 2 COMMAND: "fsstress"
#0 [ffff888080e772a0] machine_kexec at ffffffff856adedc
#1 [ffff888080e773a8] __crash_kexec at ffffffff8585cd20
#2 [ffff888080e774c0] panic at ffffffff8572b288
#3 [ffff888080e77590] oops_end at ffffffff85641f6e
#4 [ffff888080e775f0] __do_page_fault at ffffffff856cd55b
#5 [ffff888080e77668] do_page_fault at ffffffff856cd834
#6 [ffff888080e776a0] async_page_fault at ffffffff8660125e
[exception RIP: __inode_security_revalidate+34]
RIP: ffffffff85c43452 RSP: ffff888080e77758 RFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8593ae7e
RDX: 0000000000000000 RSI: 0000000000000297 RDI: 0000000000000297
RBP: ffff8881984e6628 R8: ffffed10115e3f39 R9: ffffed10115e3f39
R10: 0000000000000001 R11: ffffed10115e3f38 R12: 0000000000000001
R13: 0000000000000000 R14: ffff88808350a000 R15: 1ffff110101ceef5
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#7 [ffff888080e77780] selinux_inode_rename at ffffffff85c4418d
#8 [ffff888080e77858] security_inode_rename at ffffffff85c35f24
#9 [ffff888080e77890] vfs_rename at ffffffff85b01209
#10 [ffff888080e779a8] ovl_do_rename at ffffffffc0a44c22 [overlay]
#11 [ffff888080e779d8] ovl_rename at ffffffffc0a46575 [overlay]
#12 [ffff888080e77b48] vfs_rename at ffffffff85b0155a
#13 [ffff888080e77c60] do_renameat2 at ffffffff85b06e65
#14 [ffff888080e77f00] __x64_sys_renameat2 at ffffffff85b06fb2
#15 [ffff888080e77f30] do_syscall_64 at ffffffff85606243
#16 [ffff888080e77f50] entry_SYSCALL_64_after_hwframe at ffffffff866000ad
We can add some check in ovl_rename for this scene and return error to avoid kernel panic.
Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
---
fs/overlayfs/dir.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1fefb2b8960e..93c7c267de93 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1219,9 +1219,13 @@ static int ovl_rename(struct user_namespace *mnt_userns, struct inode *olddir,
goto out_dput;
}
} else {
- if (!d_is_negative(newdentry) &&
- (!new_opaque || !ovl_is_whiteout(newdentry)))
- goto out_dput;
+ if (!d_is_negative(newdentry)) {
+ if (!new_opaque || !ovl_is_whiteout(newdentry))
+ goto out_dput;
+ } else {
+ if (flags & RENAME_EXCHANGE)
+ goto out_dput;
+ }
}
if (olddentry == trap)
--
2.25.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ovl: fix oops in ovl_rename
2021-09-24 1:16 [PATCH] ovl: fix oops in ovl_rename Zheng Liang
@ 2021-09-24 7:12 ` 139
2021-09-24 8:50 ` zhengliang (A)
2021-09-24 19:01 ` Miklos Szeredi
1 sibling, 1 reply; 4+ messages in thread
From: 139 @ 2021-09-24 7:12 UTC (permalink / raw)
To: Zheng Liang; +Cc: miklos, linux-unionfs, yi.zhang
在 2021年9月24日，09:09，Zheng Liang <zhengliang6@huawei.com> 写道：
>
> We find a kernel NULL pointer dereference problem in overlayfs.
> The problem can appear in the following scene:
>
> mkdir lower upper work merge
> touch lower/old
> touch lower/new
> mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work merge
> rm merge/new
> ------------------------------------------------------------------------------------------
> process A(rename file in merge) process B(delete file in upper)
> renameat2(AT_FDCWD, "old", AT_FDCWD, "new", 0)
> (new is whiteout file in upper)
> do_renameat2
> vfs_rename
> ovl_rename
> (overwrite=true,ovl_lower_positive(old)=true,
> ovl_dentry_is_whiteout(new)=true)
> (we can add some delay after "flags|=RENAME_EXCHANGE",
> it can make the problem appear more easy)
> …… unlink(new)
> …… (delete whiteout in upper)
> (newdentry is negative)
> ovl_do_rename
Is it a real use case? I believe there will be many unexpected behaviors if you delete the files in upper layer deliberately.
Thanks,
Chengguang
>
> So,before commencing with ovl_do_rename that the flags maybe attach RENAME_EXCHANGE
> and the newdentry is negative in ovl_rename.If we enabled selinux,it
> will lead to kernel panic.such as the following log:
> PID: 2552045 TASK: ffff8880302faf00 CPU: 2 COMMAND: "fsstress"
> #0 [ffff888080e772a0] machine_kexec at ffffffff856adedc
> #1 [ffff888080e773a8] __crash_kexec at ffffffff8585cd20
> #2 [ffff888080e774c0] panic at ffffffff8572b288
> #3 [ffff888080e77590] oops_end at ffffffff85641f6e
> #4 [ffff888080e775f0] __do_page_fault at ffffffff856cd55b
> #5 [ffff888080e77668] do_page_fault at ffffffff856cd834
> #6 [ffff888080e776a0] async_page_fault at ffffffff8660125e
> [exception RIP: __inode_security_revalidate+34]
> RIP: ffffffff85c43452 RSP: ffff888080e77758 RFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8593ae7e
> RDX: 0000000000000000 RSI: 0000000000000297 RDI: 0000000000000297
> RBP: ffff8881984e6628 R8: ffffed10115e3f39 R9: ffffed10115e3f39
> R10: 0000000000000001 R11: ffffed10115e3f38 R12: 0000000000000001
> R13: 0000000000000000 R14: ffff88808350a000 R15: 1ffff110101ceef5
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
> #7 [ffff888080e77780] selinux_inode_rename at ffffffff85c4418d
> #8 [ffff888080e77858] security_inode_rename at ffffffff85c35f24
> #9 [ffff888080e77890] vfs_rename at ffffffff85b01209
> #10 [ffff888080e779a8] ovl_do_rename at ffffffffc0a44c22 [overlay]
> #11 [ffff888080e779d8] ovl_rename at ffffffffc0a46575 [overlay]
> #12 [ffff888080e77b48] vfs_rename at ffffffff85b0155a
> #13 [ffff888080e77c60] do_renameat2 at ffffffff85b06e65
> #14 [ffff888080e77f00] __x64_sys_renameat2 at ffffffff85b06fb2
> #15 [ffff888080e77f30] do_syscall_64 at ffffffff85606243
> #16 [ffff888080e77f50] entry_SYSCALL_64_after_hwframe at ffffffff866000ad
>
> We can add some check in ovl_rename for this scene and return error to avoid kernel panic.
>
> Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
> ---
> fs/overlayfs/dir.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 1fefb2b8960e..93c7c267de93 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1219,9 +1219,13 @@ static int ovl_rename(struct user_namespace *mnt_userns, struct inode *olddir,
> goto out_dput;
> }
> } else {
> - if (!d_is_negative(newdentry) &&
> - (!new_opaque || !ovl_is_whiteout(newdentry)))
> - goto out_dput;
> + if (!d_is_negative(newdentry)) {
> + if (!new_opaque || !ovl_is_whiteout(newdentry))
> + goto out_dput;
> + } else {
> + if (flags & RENAME_EXCHANGE)
> + goto out_dput;
> + }
> }
>
> if (olddentry == trap)
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] ovl: fix oops in ovl_rename
2021-09-24 7:12 ` 139
@ 2021-09-24 8:50 ` zhengliang (A)
0 siblings, 0 replies; 4+ messages in thread
From: zhengliang (A) @ 2021-09-24 8:50 UTC (permalink / raw)
To: 139; +Cc: miklos, linux-unionfs, zhangyi (F)
> We find a kernel NULL pointer dereference problem in overlayfs.
> The problem can appear in the following scene:
>
> mkdir lower upper work merge
> touch lower/old
> touch lower/new
> mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work
> merge rm merge/new
> ------------------------------------------------------------------------------------------
> process A(rename file in merge) process B(delete file in upper)
> renameat2(AT_FDCWD, "old", AT_FDCWD, "new", 0) (new is whiteout file
> in upper)
> do_renameat2
> vfs_rename
> ovl_rename
> (overwrite=true,ovl_lower_positive(old)=true,
> ovl_dentry_is_whiteout(new)=true)
> (we can add some delay after "flags|=RENAME_EXCHANGE",
> it can make the problem appear more easy)
> …… unlink(new)
> …… (delete whiteout in upper)
> (newdentry is negative)
> ovl_do_rename
Is it a real use case? I believe there will be many unexpected behaviors if you delete the files in upper layer deliberately.
Thanks,
Chengguang
As described in the documentation (Documentation/filesystems/overlayfs.rst)
Changes to underlying filesystems
---------------------------------
Changes to the underlying filesystems while part of a mounted overlay
filesystem are not allowed. If the underlying filesystem is changed,
the behavior of the overlay is undefined, though it will not result in
a crash or deadlock.
So, We should probably fix this crash problem.
>
> So,before commencing with ovl_do_rename that the flags maybe attach
> RENAME_EXCHANGE and the newdentry is negative in ovl_rename.If we
> enabled selinux,it will lead to kernel panic.such as the following log:
> PID: 2552045 TASK: ffff8880302faf00 CPU: 2 COMMAND: "fsstress"
> #0 [ffff888080e772a0] machine_kexec at ffffffff856adedc
> #1 [ffff888080e773a8] __crash_kexec at ffffffff8585cd20
> #2 [ffff888080e774c0] panic at ffffffff8572b288
> #3 [ffff888080e77590] oops_end at ffffffff85641f6e
> #4 [ffff888080e775f0] __do_page_fault at ffffffff856cd55b
> #5 [ffff888080e77668] do_page_fault at ffffffff856cd834
> #6 [ffff888080e776a0] async_page_fault at ffffffff8660125e
> [exception RIP: __inode_security_revalidate+34]
> RIP: ffffffff85c43452 RSP: ffff888080e77758 RFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8593ae7e
> RDX: 0000000000000000 RSI: 0000000000000297 RDI: 0000000000000297
> RBP: ffff8881984e6628 R8: ffffed10115e3f39 R9: ffffed10115e3f39
> R10: 0000000000000001 R11: ffffed10115e3f38 R12: 0000000000000001
> R13: 0000000000000000 R14: ffff88808350a000 R15: 1ffff110101ceef5
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
> #7 [ffff888080e77780] selinux_inode_rename at ffffffff85c4418d
> #8 [ffff888080e77858] security_inode_rename at ffffffff85c35f24
> #9 [ffff888080e77890] vfs_rename at ffffffff85b01209
> #10 [ffff888080e779a8] ovl_do_rename at ffffffffc0a44c22 [overlay]
> #11 [ffff888080e779d8] ovl_rename at ffffffffc0a46575 [overlay]
> #12 [ffff888080e77b48] vfs_rename at ffffffff85b0155a
> #13 [ffff888080e77c60] do_renameat2 at ffffffff85b06e65
> #14 [ffff888080e77f00] __x64_sys_renameat2 at ffffffff85b06fb2
> #15 [ffff888080e77f30] do_syscall_64 at ffffffff85606243
> #16 [ffff888080e77f50] entry_SYSCALL_64_after_hwframe at
> ffffffff866000ad
>
> We can add some check in ovl_rename for this scene and return error to avoid kernel panic.
>
> Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
> ---
> fs/overlayfs/dir.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index
> 1fefb2b8960e..93c7c267de93 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1219,9 +1219,13 @@ static int ovl_rename(struct user_namespace *mnt_userns, struct inode *olddir,
> goto out_dput;
> }
> } else {
> - if (!d_is_negative(newdentry) &&
> - (!new_opaque || !ovl_is_whiteout(newdentry)))
> - goto out_dput;
> + if (!d_is_negative(newdentry)) {
> + if (!new_opaque || !ovl_is_whiteout(newdentry))
> + goto out_dput;
> + } else {
> + if (flags & RENAME_EXCHANGE)
> + goto out_dput;
> + }
> }
>
> if (olddentry == trap)
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ovl: fix oops in ovl_rename
2021-09-24 1:16 [PATCH] ovl: fix oops in ovl_rename Zheng Liang
2021-09-24 7:12 ` 139
@ 2021-09-24 19:01 ` Miklos Szeredi
1 sibling, 0 replies; 4+ messages in thread
From: Miklos Szeredi @ 2021-09-24 19:01 UTC (permalink / raw)
To: Zheng Liang; +Cc: overlayfs, zhangyi (F)
On Fri, 24 Sept 2021 at 03:09, Zheng Liang <zhengliang6@huawei.com> wrote:
>
> We find a kernel NULL pointer dereference problem in overlayfs.
> The problem can appear in the following scene:
>
> mkdir lower upper work merge
> touch lower/old
> touch lower/new
> mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work merge
> rm merge/new
> ------------------------------------------------------------------------------------------
> process A(rename file in merge) process B(delete file in upper)
> renameat2(AT_FDCWD, "old", AT_FDCWD, "new", 0)
> (new is whiteout file in upper)
> do_renameat2
> vfs_rename
> ovl_rename
> (overwrite=true,ovl_lower_positive(old)=true,
> ovl_dentry_is_whiteout(new)=true)
> (we can add some delay after "flags|=RENAME_EXCHANGE",
> it can make the problem appear more easy)
> …… unlink(new)
> …… (delete whiteout in upper)
> (newdentry is negative)
> ovl_do_rename
>
> So,before commencing with ovl_do_rename that the flags maybe attach RENAME_EXCHANGE
> and the newdentry is negative in ovl_rename.If we enabled selinux,it
> will lead to kernel panic.such as the following log:
> PID: 2552045 TASK: ffff8880302faf00 CPU: 2 COMMAND: "fsstress"
> #0 [ffff888080e772a0] machine_kexec at ffffffff856adedc
> #1 [ffff888080e773a8] __crash_kexec at ffffffff8585cd20
> #2 [ffff888080e774c0] panic at ffffffff8572b288
> #3 [ffff888080e77590] oops_end at ffffffff85641f6e
> #4 [ffff888080e775f0] __do_page_fault at ffffffff856cd55b
> #5 [ffff888080e77668] do_page_fault at ffffffff856cd834
> #6 [ffff888080e776a0] async_page_fault at ffffffff8660125e
> [exception RIP: __inode_security_revalidate+34]
> RIP: ffffffff85c43452 RSP: ffff888080e77758 RFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8593ae7e
> RDX: 0000000000000000 RSI: 0000000000000297 RDI: 0000000000000297
> RBP: ffff8881984e6628 R8: ffffed10115e3f39 R9: ffffed10115e3f39
> R10: 0000000000000001 R11: ffffed10115e3f38 R12: 0000000000000001
> R13: 0000000000000000 R14: ffff88808350a000 R15: 1ffff110101ceef5
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
> #7 [ffff888080e77780] selinux_inode_rename at ffffffff85c4418d
> #8 [ffff888080e77858] security_inode_rename at ffffffff85c35f24
> #9 [ffff888080e77890] vfs_rename at ffffffff85b01209
> #10 [ffff888080e779a8] ovl_do_rename at ffffffffc0a44c22 [overlay]
> #11 [ffff888080e779d8] ovl_rename at ffffffffc0a46575 [overlay]
> #12 [ffff888080e77b48] vfs_rename at ffffffff85b0155a
> #13 [ffff888080e77c60] do_renameat2 at ffffffff85b06e65
> #14 [ffff888080e77f00] __x64_sys_renameat2 at ffffffff85b06fb2
> #15 [ffff888080e77f30] do_syscall_64 at ffffffff85606243
> #16 [ffff888080e77f50] entry_SYSCALL_64_after_hwframe at ffffffff866000ad
>
> We can add some check in ovl_rename for this scene and return error to avoid kernel panic.
Thanks. Patch looks good; pushed to vfs.git#overlayfs-next (with a
cleaned up commit message).
Miklos
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-24 19:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 1:16 [PATCH] ovl: fix oops in ovl_rename Zheng Liang
2021-09-24 7:12 ` 139
2021-09-24 8:50 ` zhengliang (A)
2021-09-24 19:01 ` Miklos Szeredi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).