diff mbox series

btrfs: do not allow non subvolume root targets for snapshot

Message ID 27a3901ec5c2f63650441e5c99f430fce864b609.1702652494.git.josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series btrfs: do not allow non subvolume root targets for snapshot | expand

Commit Message

Josef Bacik Dec. 15, 2023, 3:01 p.m. UTC
Our btrfs subvolume snapshot <source> <destination> utility enforces
that <source> is the root of the subvolume, however this isn't enforced
in the kernel.  Update the kernel to also enforce this limitation to
avoid problems with other users of this ioctl that don't have the
appropriate checks in place.

cc: stable@vger.kernel.org
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ioctl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Neal Gompa Dec. 15, 2023, 9:02 p.m. UTC | #1
On Fri, Dec 15, 2023 at 10:02 AM Josef Bacik <josef@toxicpanda.com> wrote:
>
> Our btrfs subvolume snapshot <source> <destination> utility enforces
> that <source> is the root of the subvolume, however this isn't enforced
> in the kernel.  Update the kernel to also enforce this limitation to
> avoid problems with other users of this ioctl that don't have the
> appropriate checks in place.
>
> cc: stable@vger.kernel.org
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/ioctl.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 4e50b62db2a8..298edca43901 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1290,6 +1290,16 @@ static noinline int __btrfs_ioctl_snap_create(struct file *file,
>                          * are limited to own subvolumes only
>                          */
>                         ret = -EPERM;
> +               } else if (btrfs_ino(BTRFS_I(src_inode)) !=
> +                          BTRFS_FIRST_FREE_OBJECTID) {
> +                       /*
> +                        * Snapshots must be made with the src_inode referring
> +                        * to the subvolume inode, otherwise the permission
> +                        * checking above is useless because we may have
> +                        * permission on a lower diretory but not the subvol
> +                        * itself.
> +                        */
> +                       ret = -EINVAL;
>                 } else {
>                         ret = btrfs_mksnapshot(&file->f_path, idmap,
>                                                name, namelen,
> --
> 2.43.0
>
>

Yes, please!

Reviewed-by: Neal Gompa <neal@gompa.dev>
diff mbox series

Patch

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 4e50b62db2a8..298edca43901 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1290,6 +1290,16 @@  static noinline int __btrfs_ioctl_snap_create(struct file *file,
 			 * are limited to own subvolumes only
 			 */
 			ret = -EPERM;
+		} else if (btrfs_ino(BTRFS_I(src_inode)) !=
+			   BTRFS_FIRST_FREE_OBJECTID) {
+			/*
+			 * Snapshots must be made with the src_inode referring
+			 * to the subvolume inode, otherwise the permission
+			 * checking above is useless because we may have
+			 * permission on a lower diretory but not the subvol
+			 * itself.
+			 */
+			ret = -EINVAL;
 		} else {
 			ret = btrfs_mksnapshot(&file->f_path, idmap,
 					       name, namelen,