On Tue, 26 Oct 2010, Goffredo Baroncelli wrote:
> >     inode = dentry->d_inode;
> > +   dest = BTRFS_I(inode)->root;
> > +   if (!capable(CAP_SYS_ADMIN)){
> > +           /*
> > +            * Regular user.  Only allow this with a special mount
> > +            * option, and when rmdir(2) would have been allowed.
> > +            *
> > +            * Note that this is _not_ check that the subvol is
> > +            * empty or doesn't contain data that we wouldn't
> > +            * otherwise be able to delete.
> > +            *
> > +            * Users who want to delete empty subvols should try
> > +            * rmdir(2).
> > +            */
> > +           err = -EPERM;
> > +           if (!btrfs_test_opt(root, USER_SUBVOL_RM_ALLOWED))
> > +                   goto out_dput;
> > +
> > +           /*
> > +            * Do not allow deletion if the parent dir is the same
> > +            * as the dir to be deleted.  That means the ioctl
> > +            * must be called on the dentry referencing the root
> > +            * of the subvol, not a random directory contained
> > +            * within it.
> > +            */
> > +           err = -EINVAL;
> > +           if (root == dest)
> > +                   goto out_dput;
> > +
> > +           /* check if subvolume may be deleted by a non-root user */      
> > +           err = btrfs_may_delete(dir, dentry, 1);
> > +           if (err)
> > +                   goto out_dput;
> 
> If I read correctly, an user now is capable to remove a "not owned" 
> subvolume. 
> Is this the intended behavior ?
> I am not arguing against, but I want to highlight this fact.

Good point.  This was mirroring the rmdir(2) checks, but given that we can 
remove a non-empty subvol, we should add

+               err = inode_permission(inode, MAY_WRITE | MAY_EXEC);
+               if (err)
+                       goto out_dput;

as well.  I'll resend.

Thanks!
sage


> 
> 
> > +   }
> > +
> >     if (inode->i_ino != BTRFS_FIRST_FREE_OBJECTID) {
> >             err = -EINVAL;
> >             goto out_dput;
> >     }
> >  
> > -   dest = BTRFS_I(inode)->root;
> > -
> >     mutex_lock(&inode->i_mutex);
> >     err = d_invalidate(dentry);
> >     if (err)
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 4da2680..8ff5a3a 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -68,7 +68,7 @@ enum {
> >     Opt_nodatacow, Opt_max_inline, Opt_alloc_start, Opt_nobarrier, 
> Opt_ssd,
> >     Opt_nossd, Opt_ssd_spread, Opt_thread_pool, Opt_noacl, Opt_compress,
> >     Opt_compress_force, Opt_notreelog, Opt_ratio, Opt_flushoncommit,
> > -   Opt_discard, Opt_err,
> > +   Opt_discard, Opt_user_subvol_rm_allowed, Opt_err,
> >  };
> >  
> >  static match_table_t tokens = {
> > @@ -92,6 +92,7 @@ static match_table_t tokens = {
> >     {Opt_flushoncommit, "flushoncommit"},
> >     {Opt_ratio, "metadata_ratio=%d"},
> >     {Opt_discard, "discard"},
> > +   {Opt_user_subvol_rm_allowed, "user_subvol_rm_allowed"},
> >     {Opt_err, NULL},
> >  };
> >  
> > @@ -235,6 +236,9 @@ int btrfs_parse_options(struct btrfs_root *root, char 
> *options)
> >             case Opt_discard:
> >                     btrfs_set_opt(info->mount_opt, DISCARD);
> >                     break;
> > +           case Opt_user_subvol_rm_allowed:
> > +                   btrfs_set_opt(info->mount_opt, 
> USER_SUBVOL_RM_ALLOWED);
> > +                   break;
> >             case Opt_err:
> >                     printk(KERN_INFO "btrfs: unrecognized mount option "
> >                            "'%s'\n", p);
> > -- 
> > 1.6.6.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <kreij...@inwind.it>
> Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to