| Message ID | 20230811014435.1963948-1-lee@trager.us (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Copy dir permission and time when creating a stub subvolume | expand |
On Thu, Aug 10, 2023 at 06:44:35PM -0700, Lee Trager wrote: > btrfs supports creating nested subvolumes however snapshots are not recursive. > When a snapshot is taken of a volume which contains a subvolume the subvolume > is replaced with a stub subvolume which has the same name and uses inode > number 2[1]. The stub subvolume kept the directory name but did not set the > time or permissions of the stub subvolume. This resulted in all time information > being the current time and ownership defaulting to root. When subvolumes and > snapshots are created using unshare this results in a snapshot directory the > user created but has no permissions for. > > Test case: > [vmuser@archvm ~]# sudo -i > [root@archvm ~]# mkdir -p /mnt/btrfs/test > [root@archvm ~]# chown vmuser:users /mnt/btrfs/test/ > [root@archvm ~]# exit > logout > [vmuser@archvm ~]$ cd /mnt/btrfs/test > [vmuser@archvm test]$ unshare --user --keep-caps --map-auto --map-root-user > [root@archvm test]# btrfs subvolume create subvolume > Create subvolume './subvolume' > [root@archvm test]# btrfs subvolume create subvolume/subsubvolume > Create subvolume 'subvolume/subsubvolume' > [root@archvm test]# btrfs subvolume snapshot subvolume snapshot > Create a snapshot of 'subvolume' in './snapshot' > [root@archvm test]# exit > logout > [vmuser@archvm test]$ tree -ug > [vmuser users ] . > ├── [vmuser users ] snapshot > │ └── [vmuser users ] subsubvolume <-- Without patch perm is root:root > └── [vmuser users ] subvolume > └── [vmuser users ] subsubvolume > > 5 directories, 0 files > > [1] https://btrfs.readthedocs.io/en/latest/btrfs-subvolume.html#nested-subvolumes > Signed-off-by: Lee Trager <lee@trager.us> > --- > fs/btrfs/inode.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 6a68d5a3ed20..7a288cd6f815 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -5592,11 +5592,11 @@ struct inode *btrfs_iget(struct super_block *s, u64 ino, struct btrfs_root *root > return btrfs_iget_path(s, ino, root, NULL); > } > > -static struct inode *new_simple_dir(struct super_block *s, > +static struct inode *new_simple_dir(struct inode *dir, > struct btrfs_key *key, > struct btrfs_root *root) > { > - struct inode *inode = new_inode(s); > + struct inode *inode = new_inode(dir->i_sb); > > if (!inode) > return ERR_PTR(-ENOMEM); > @@ -5615,9 +5615,11 @@ static struct inode *new_simple_dir(struct super_block *s, > inode->i_fop = &simple_dir_operations; > inode->i_mode = S_IFDIR | S_IRUGO | S_IWUSR | S_IXUGO; > inode->i_mtime = current_time(inode); > - inode->i_atime = inode->i_mtime; > - inode->i_ctime = inode->i_mtime; > + inode->i_atime = dir->i_atime; > + inode->i_ctime = dir->i_ctime; > BTRFS_I(inode)->i_otime = inode->i_mtime; > + inode->i_uid = dir->i_uid; > + inode->i_gid = dir->i_gid; The uid and git are subject to namespaces so it's usually read as i_uid_read/i_gid_read. Howvever in this case it's 1:1 copy (and not passing arguments or storing to something else than another inode), so I guess it's correct. Added to misc-next, thanks.
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 6a68d5a3ed20..7a288cd6f815 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5592,11 +5592,11 @@ struct inode *btrfs_iget(struct super_block *s, u64 ino, struct btrfs_root *root return btrfs_iget_path(s, ino, root, NULL); } -static struct inode *new_simple_dir(struct super_block *s, +static struct inode *new_simple_dir(struct inode *dir, struct btrfs_key *key, struct btrfs_root *root) { - struct inode *inode = new_inode(s); + struct inode *inode = new_inode(dir->i_sb); if (!inode) return ERR_PTR(-ENOMEM); @@ -5615,9 +5615,11 @@ static struct inode *new_simple_dir(struct super_block *s, inode->i_fop = &simple_dir_operations; inode->i_mode = S_IFDIR | S_IRUGO | S_IWUSR | S_IXUGO; inode->i_mtime = current_time(inode); - inode->i_atime = inode->i_mtime; - inode->i_ctime = inode->i_mtime; + inode->i_atime = dir->i_atime; + inode->i_ctime = dir->i_ctime; BTRFS_I(inode)->i_otime = inode->i_mtime; + inode->i_uid = dir->i_uid; + inode->i_gid = dir->i_gid; return inode; } @@ -5676,7 +5678,7 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry) if (ret != -ENOENT) inode = ERR_PTR(ret); else - inode = new_simple_dir(dir->i_sb, &location, root); + inode = new_simple_dir(dir, &location, root); } else { inode = btrfs_iget(dir->i_sb, location.objectid, sub_root); btrfs_put_root(sub_root);
btrfs supports creating nested subvolumes however snapshots are not recursive. When a snapshot is taken of a volume which contains a subvolume the subvolume is replaced with a stub subvolume which has the same name and uses inode number 2[1]. The stub subvolume kept the directory name but did not set the time or permissions of the stub subvolume. This resulted in all time information being the current time and ownership defaulting to root. When subvolumes and snapshots are created using unshare this results in a snapshot directory the user created but has no permissions for. Test case: [vmuser@archvm ~]# sudo -i [root@archvm ~]# mkdir -p /mnt/btrfs/test [root@archvm ~]# chown vmuser:users /mnt/btrfs/test/ [root@archvm ~]# exit logout [vmuser@archvm ~]$ cd /mnt/btrfs/test [vmuser@archvm test]$ unshare --user --keep-caps --map-auto --map-root-user [root@archvm test]# btrfs subvolume create subvolume Create subvolume './subvolume' [root@archvm test]# btrfs subvolume create subvolume/subsubvolume Create subvolume 'subvolume/subsubvolume' [root@archvm test]# btrfs subvolume snapshot subvolume snapshot Create a snapshot of 'subvolume' in './snapshot' [root@archvm test]# exit logout [vmuser@archvm test]$ tree -ug [vmuser users ] . ├── [vmuser users ] snapshot │ └── [vmuser users ] subsubvolume <-- Without patch perm is root:root └── [vmuser users ] subvolume └── [vmuser users ] subsubvolume 5 directories, 0 files [1] https://btrfs.readthedocs.io/en/latest/btrfs-subvolume.html#nested-subvolumes Signed-off-by: Lee Trager <lee@trager.us> --- fs/btrfs/inode.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)