ACK/CMNT: [PATCH][SRU][Disco] UBUNTU: SAUCE: shiftfs: use separate llseek method for directories

[Tyler Hicks]

On 2019-04-19 18:39:46, Christian Brauner wrote:
> On Fri, Apr 19, 2019 at 11:34:45AM -0500, Tyler Hicks wrote:
> > On 2019-04-16 18:29:50, Christian Brauner wrote:
> > > BugLink: https://bugs.launchpad.net/bugs/1824812
> > > 
> > > Give shiftfs it's own proper llseek method for directories.
> > > 
> > > Before this commit we used to rely on an llseek method that was targeted
> > > for regular files for both directories and regular files. Give
> > > directories their own llseek operation.
> > > This is required to have AppArmor function correctly with shiftfs.
> > 
> > I think the commit message should be improved a little bit. This was a
> > quick one that was thrown together while we were urgently debugging the
> > bug report.
> > 
> > The commit message is a bit misleading because the commit doesn't
> > specifically fix AppArmor + shiftfs. It fixes shiftfs, in general,
> > since it is common for userspace to expect lseek(2) to work on directory
> > files. How about this:
> > 
> >   Give shiftfs it's own proper llseek method for directories.
> > 
> >   Before this commit we used to rely on an llseek method that was
> >   targeted for regular files for both directories and regular files.
> >   However, the realfile's f_pos was not correctly handled when userspace
> >   called lseek(2) on a shiftfs directory file. Give directories their
> >   own llseek operation so that seeking on a directory file is properly
> >   supported.
> > 
> > I think that'll allow the commit message to age a little better once
> > we've all forgotten the details of this bug. With that change,
> > 
> > Acked-by: Tyler Hicks <tyhicks at canonical.com>
> 
> Sure, can you just drop-in your commit message instead of mine when you
> apply, please? Or do you want me to resend?

I think the stable kernel team will drop-in my suggested commit message
when they gather up and apply the acks. I don't think there's a need for
you to resend.

Tyler

> 
> > 
> > Tyler
> > 
> > > 
> > > Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
> > > ---
> > >  fs/shiftfs.c | 14 +++++++++++---
> > >  1 file changed, 11 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/fs/shiftfs.c b/fs/shiftfs.c
> > > index 4c8a6ec2a617..9771165d1ce0 100644
> > > --- a/fs/shiftfs.c
> > > +++ b/fs/shiftfs.c
> > > @@ -1144,7 +1144,15 @@ static int shiftfs_release(struct inode *inode, struct file *file)
> > >  	return 0;
> > >  }
> > >  
> > > -static loff_t shiftfs_llseek(struct file *file, loff_t offset, int whence)
> > > +static loff_t shiftfs_dir_llseek(struct file *file, loff_t offset, int whence)
> > > +{
> > > +	struct shiftfs_file_info *file_info = file->private_data;
> > > +	struct file *realfile = file_info->realfile;
> > > +
> > > +	return vfs_llseek(realfile, offset, whence);
> > > +}
> > > +
> > > +static loff_t shiftfs_file_llseek(struct file *file, loff_t offset, int whence)
> > >  {
> > >  	struct inode *realinode = file_inode(file)->i_private;
> > >  
> > > @@ -1653,7 +1661,7 @@ static int shiftfs_iterate_shared(struct file *file, struct dir_context *ctx)
> > >  const struct file_operations shiftfs_file_operations = {
> > >  	.open			= shiftfs_open,
> > >  	.release		= shiftfs_release,
> > > -	.llseek			= shiftfs_llseek,
> > > +	.llseek			= shiftfs_file_llseek,
> > >  	.read_iter		= shiftfs_read_iter,
> > >  	.write_iter		= shiftfs_write_iter,
> > >  	.fsync			= shiftfs_fsync,
> > > @@ -1670,7 +1678,7 @@ const struct file_operations shiftfs_dir_operations = {
> > >  	.compat_ioctl		= shiftfs_compat_ioctl,
> > >  	.fsync			= shiftfs_fsync,
> > >  	.iterate_shared		= shiftfs_iterate_shared,
> > > -	.llseek			= shiftfs_llseek,
> > > +	.llseek			= shiftfs_dir_llseek,
> > >  	.open			= shiftfs_open,
> > >  	.read			= generic_read_dir,
> > >  	.release		= shiftfs_release,
> > > -- 
> > > 2.21.0
> > > 
> > > 
> > > -- 
> > > kernel-team mailing list
> > > kernel-team at lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team