[PATCH 5.10 0/2] Backport two overlayfs fixed to 5.10.y

[PATCH 5.10 0/2] Backport two overlayfs fixed to 5.10.y

[Amir Goldstein]

Greg,

These two patches have been (correctly) auto selected to 5.15.y
along with the two dependency patches tagged with:
Stable-dep-of: b306e90ffabd ("ovl: remove privs in ovl_copyfile()")
9636e70ee2d3 ("ovl: use ovl_copy_{real,upper}attr() wrappers")
a54843833caf ("ovl: store lower path in ovl_inode")

It wasn't wrong to apply those patches with the two dependencies
to 5.15.y, but it is not as easy to do for 5.10.y, so here is a
very simple backport of the two fixes to 5.10.y, i.e.:
replaced ovl_copyattr(X) with ovl_copyattr(ovl_inode_real(X), X).

Note that the language "This fixes some failure in fstests..."
in commit message means that those fixes are not enough for the
tests to pass. Additional backports from v6.2 are needed for the
tests to pass and I am collaborating those backports with Leah,
so they will hit 5.15.y first before posting them for 5.10.y.

Never the less, these overlayfs fixes are important security
fixes, so they should be applied to LTS kernel even before
all the cases in the fstests are fixed.

Thanks,
Amir.

Amir Goldstein (2):
  ovl: remove privs in ovl_copyfile()
  ovl: remove privs in ovl_fallocate()

 fs/overlayfs/file.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 5.10 1/2] ovl: remove privs in ovl_copyfile()

[Amir Goldstein]

commit b306e90ffabdaa7e3b3350dbcd19b7663e71ab17 upstream.

Underlying fs doesn't remove privs because copy_range/remap_range are
called with privileged mounter credentials.

This fixes some failures in fstest generic/673.

Fixes: 8ede205541ff ("ovl: add reflink/copyfile/dedup support")
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index b019f27c1360..259b2d41b707 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -687,14 +687,23 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 	const struct cred *old_cred;
 	loff_t ret;
 
+	inode_lock(inode_out);
+	if (op != OVL_DEDUPE) {
+		/* Update mode */
+		ovl_copyattr(ovl_inode_real(inode_out), inode_out);
+		ret = file_remove_privs(file_out);
+		if (ret)
+			goto out_unlock;
+	}
+
 	ret = ovl_real_fdget(file_out, &real_out);
 	if (ret)
-		return ret;
+		goto out_unlock;
 
 	ret = ovl_real_fdget(file_in, &real_in);
 	if (ret) {
 		fdput(real_out);
-		return ret;
+		goto out_unlock;
 	}
 
 	old_cred = ovl_override_creds(file_inode(file_out)->i_sb);
@@ -723,6 +732,9 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 	fdput(real_in);
 	fdput(real_out);
 
+out_unlock:
+	inode_unlock(inode_out);
+
 	return ret;
 }
 
-- 
2.34.1

[PATCH 5.10 2/2] ovl: remove privs in ovl_fallocate()

[Amir Goldstein]

commit 23a8ce16419a3066829ad4a8b7032a75817af65b upstream.

Underlying fs doesn't remove privs because fallocate is called with
privileged mounter credentials.

This fixes some failure in fstests generic/683..687.

Fixes: aab8848cee5e ("ovl: add ovl_fallocate()")
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 259b2d41b707..0e734c8b4dfa 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -531,9 +531,16 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 	const struct cred *old_cred;
 	int ret;
 
+	inode_lock(inode);
+	/* Update mode */
+	ovl_copyattr(ovl_inode_real(inode), inode);
+	ret = file_remove_privs(file);
+	if (ret)
+		goto out_unlock;
+
 	ret = ovl_real_fdget(file, &real);
 	if (ret)
-		return ret;
+		goto out_unlock;
 
 	old_cred = ovl_override_creds(file_inode(file)->i_sb);
 	ret = vfs_fallocate(real.file, mode, offset, len);
@@ -544,6 +551,9 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 
 	fdput(real);
 
+out_unlock:
+	inode_unlock(inode);
+
 	return ret;
 }
 
-- 
2.34.1

Re: [PATCH 5.10 0/2] Backport two overlayfs fixed to 5.10.y

[Greg Kroah-Hartman]

On Sun, Feb 12, 2023 at 11:02:02AM +0200, Amir Goldstein wrote:
> Greg,
> 
> These two patches have been (correctly) auto selected to 5.15.y
> along with the two dependency patches tagged with:
> Stable-dep-of: b306e90ffabd ("ovl: remove privs in ovl_copyfile()")
> 9636e70ee2d3 ("ovl: use ovl_copy_{real,upper}attr() wrappers")
> a54843833caf ("ovl: store lower path in ovl_inode")
> 
> It wasn't wrong to apply those patches with the two dependencies
> to 5.15.y, but it is not as easy to do for 5.10.y, so here is a
> very simple backport of the two fixes to 5.10.y, i.e.:
> replaced ovl_copyattr(X) with ovl_copyattr(ovl_inode_real(X), X).
> 
> Note that the language "This fixes some failure in fstests..."
> in commit message means that those fixes are not enough for the
> tests to pass. Additional backports from v6.2 are needed for the
> tests to pass and I am collaborating those backports with Leah,
> so they will hit 5.15.y first before posting them for 5.10.y.
> 
> Never the less, these overlayfs fixes are important security
> fixes, so they should be applied to LTS kernel even before
> all the cases in the fstests are fixed.
> 
> Thanks,
> Amir.
> 
> Amir Goldstein (2):
>   ovl: remove privs in ovl_copyfile()
>   ovl: remove privs in ovl_fallocate()
> 
>  fs/overlayfs/file.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> -- 
> 2.34.1
> 

Now queued up, thanks.

greg k-h