On Mon, Jul 24, 2017 at 10:02:30AM -0400, Josef Bacik wrote:
> On Mon, Jul 24, 2017 at 02:42:29PM +0200, David Sterba wrote:
> > On Fri, Jul 21, 2017 at 01:29:07PM -0400, [email protected] wrote:
> > > From: Josef Bacik <[email protected]>
> > > 
> > > We need to use file->private_data for readdir on directories, so just
> > > don't allow user space transactions on directories.
> > > 
> > > Signed-off-by: Josef Bacik <[email protected]>
> > > ---
> > >  fs/btrfs/ioctl.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > > index bedeec6..ddb3811 100644
> > > --- a/fs/btrfs/ioctl.c
> > > +++ b/fs/btrfs/ioctl.c
> > > @@ -3968,6 +3968,9 @@ static long btrfs_ioctl_trans_start(struct file 
> > > *file)
> > >   struct btrfs_trans_handle *trans;
> > >   int ret;
> > >  
> > > + if (S_ISDIR(inode->i_mode))
> > > +         return -EINVAL;
> > 
> > You can't do this, starting a transaction on a directory needs to work.
> > The most natural way to run the ioctl is on the mount point.
> > 
> > The file private data would need to be able to hold multipe values, so
> > you can add
> > 
> > struct btrfs_inode {
> >     ...
> >     struct priv_data {
> >             void *for_readdir;
> >             void *for_tranc_ioctl;
> >     };
> >     ...
> > };
> > 
> > then set file->file_private = &btrfs_inode->priv_data; and update all
> > uses to check for the embedded pointers.
> 
> Blah I really want to just jetison the user space transaction stuff altogether
> so I was hoping this would be a first step.  But yeah we can do it your way 
> too.

I'm fine with removing the trans ioctl, ceph does not use it. We may
need one or two release cycles when we mark it deprecated and
WARN_ON_ONCE when used. But as it's undocumented and tricky to use I
guess nobody will notice.  Unfortunatelly this means you still have to
add the extra structures for readdir.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to