[PATCH 0/4] btrfs: drop path before copying to userspace

[PATCH 0/4] btrfs: drop path before copying to userspace

[Anand Jain]

In the ioctl functions below, we are holing ref to the path when copying
the temp buffer to the userland. Which can lead to a similar lock splat
warning as in the commit
 [PATCH] btrfs: drop path before copying root refs to userspace

 btrfs_ioctl_logical_to_ino
 btrfs_ioctl_ino_to_path
 btrfs_ioctl_get_subvol_rootref
 btrfs_ioctl_get_subvol_info

Fix this by freeing the path before we copy it to userspace.

Individual patch 4/4 is also in the ML and is different from here: Check
the value of ret to copy got dropped to keep it closer to the original
logic. However, its version is unchanged to match the rest of the patch
version.

Anand Jain (4):
  btrfs: drop path before copying inodes to userspace
  btrfs: drop path before copying fspath to userspace
  btrfs: drop path before copying rootrefs to userspace
  btrfs: drop path before copying subvol info to userspace

 fs/btrfs/ioctl.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

-- 
2.31.1

[PATCH 1/4] btrfs: drop path before copying inodes to userspace

[Anand Jain]

btrfs_ioctl_logical_to_ino() frees the search path after the userspace
copy from the temp buffer %inodes. Which potentially can lead to a lock
splat.

Fix this by freeing the path before we copy %inodes to userspace.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a64a71d882dc..76198c7203f1 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3391,21 +3391,20 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_fs_info *fs_info,
 		size = min_t(u32, loi->size, SZ_16M);
 	}
 
-	path = btrfs_alloc_path();
-	if (!path) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
 	inodes = init_data_container(size);
 	if (IS_ERR(inodes)) {
 		ret = PTR_ERR(inodes);
-		inodes = NULL;
-		goto out;
+		goto out_loi;
 	}
 
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		goto out;
+	}
 	ret = iterate_inodes_from_logical(loi->logical, fs_info, path,
 					  inodes, ignore_offset);
+	btrfs_free_path(path);
 	if (ret == -EINVAL)
 		ret = -ENOENT;
 	if (ret < 0)
@@ -3417,7 +3416,6 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_fs_info *fs_info,
 		ret = -EFAULT;
 
 out:
-	btrfs_free_path(path);
 	kvfree(inodes);
 out_loi:
 	kfree(loi);
-- 
2.31.1

[PATCH 2/4] btrfs: drop path before copying fspath to userspace

[Anand Jain]

btrfs_ioctl_ino_to_path() frees the search path after the userspace copy
from the temp buffer %ipath->fspath. Which potentially can lead to a lock
splat warning.

Fix this by freeing the path before we copy it to userspace.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 76198c7203f1..707b2321c8db 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3341,6 +3341,8 @@ static long btrfs_ioctl_ino_to_path(struct btrfs_root *root, void __user *arg)
 		ipath->fspath->val[i] = rel_ptr;
 	}
 
+	btrfs_free_path(path);
+	path = NULL;
 	ret = copy_to_user((void __user *)(unsigned long)ipa->fspath,
 			   ipath->fspath, size);
 	if (ret) {
-- 
2.31.1

[PATCH 3/4] btrfs: drop path before copying rootrefs to userspace

[Anand Jain]

btrfs_ioctl_get_subvol_rootref() frees the search path after the userspace
copy from the temp buffer %rootrefs, which can lead to a lock splat warning.

Fix this by freeing the path before we copy it to userspace.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 707b2321c8db..ec310868591c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2303,6 +2303,7 @@ static int btrfs_ioctl_get_subvol_rootref(struct btrfs_root *root,
 	}
 
 out:
+	btrfs_free_path(path);
 	if (!ret || ret == -EOVERFLOW) {
 		rootrefs->num_items = found;
 		/* update min_treeid for next search */
@@ -2314,7 +2315,6 @@ static int btrfs_ioctl_get_subvol_rootref(struct btrfs_root *root,
 	}
 
 	kfree(rootrefs);
-	btrfs_free_path(path);
 
 	return ret;
 }
-- 
2.31.1

[PATCH 4/4] btrfs: drop path before copying subvol info to userspace

[Anand Jain]

btrfs_ioctl_get_subvol_info() frees the search path after the userspace
copy from the temp buffer %subvol_info. This can lead to a lock splat
warning.

Fix this by freeing the path before we copy it to userspace.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ec310868591c..18be82a4d01b 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2214,6 +2214,8 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
 		}
 	}
 
+	btrfs_free_path(path);
+	path = NULL;
 	if (copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
 		ret = -EFAULT;
 
-- 
2.31.1

Re: [PATCH 0/4] btrfs: drop path before copying to userspace

[David Sterba]

On Thu, Nov 10, 2022 at 11:36:27AM +0530, Anand Jain wrote:
> In the ioctl functions below, we are holing ref to the path when copying
> the temp buffer to the userland. Which can lead to a similar lock splat
> warning as in the commit
>  [PATCH] btrfs: drop path before copying root refs to userspace
> 
>  btrfs_ioctl_logical_to_ino
>  btrfs_ioctl_ino_to_path
>  btrfs_ioctl_get_subvol_rootref
>  btrfs_ioctl_get_subvol_info
> 
> Fix this by freeing the path before we copy it to userspace.
> 
> Individual patch 4/4 is also in the ML and is different from here: Check
> the value of ret to copy got dropped to keep it closer to the original
> logic. However, its version is unchanged to match the rest of the patch
> version.
> 
> Anand Jain (4):
>   btrfs: drop path before copying inodes to userspace
>   btrfs: drop path before copying fspath to userspace
>   btrfs: drop path before copying rootrefs to userspace
>   btrfs: drop path before copying subvol info to userspace

Patch 3 dropped as it was sent by Josef. The rest added to misc-next,
thanks.