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

[Tyler Hicks]

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>

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