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, josef@xxxxxxxxxxxxxx wrote:
> > > From: Josef Bacik <jbacik@xxxxxx>
> > >
> > > 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 <jbacik@xxxxxx>
> > > ---
> > > 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
