ACK/CMNT: [PATCH 1/2][SRU][Disco] UBUNTU: SAUCE: shiftfs: use translated ids when chaning lower fs attrs

[Tyler Hicks]

[+Christian]

On 2019-04-11 09:04:24, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1824350
> 
> shiftfs_setattr() is preparing a new set of attributes with the
> owner translated for the lower fs, but it then passes the
> original attrs. As a result the owner is set to the untranslated
> owner, which causes the shiftfs inodes to also have incorrect
> ids. For example:
> 
>  # mkdir dir
>  # touch file
>  # ls -lh dir file
>  drwxr-xr-x 2 root root 4.0K Apr 11 13:05 dir
>  -rw-r--r-- 1 root root 0 Apr 11 13:05 file
>  # chown 500:500 dir file
>  # ls -lh dir file
>  drwxr-xr-x 2 1000500 1000500 4.0K Apr 11 12:42 dir
>  -rw-r--r-- 1 1000500 1000500 0 Apr 11 12:42 file
> 
> Fix this to pass the correct iattr struct to notify_change().
> 
> Reviewed-by: Christian Brauner <christian.brauner at ubuntu.com>
> Signed-off-by: Seth Forshee <seth.forshee at canonical.com>

Acked-by: Tyler Hicks <tyhicks at canonical.com>


I took another look at shiftfs_setattr(). Doesn't ia_file need to be
translated to a lower file when (newattr.ia_valid & ATTR_FILE) is
true? Otherwise, we're giving a file with shiftfs_{file,dir}_operations
to the lower filesystem's setattr operation.

Tyler

> ---
>  fs/shiftfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/shiftfs.c b/fs/shiftfs.c
> index 678cad30f4a5..e736fd6afcb4 100644
> --- a/fs/shiftfs.c
> +++ b/fs/shiftfs.c
> @@ -779,7 +779,7 @@ static int shiftfs_setattr(struct dentry *dentry, struct iattr *attr)
>  
>  	inode_lock(loweri);
>  	oldcred = shiftfs_override_creds(dentry->d_sb);
> -	err = notify_change(lowerd, attr, NULL);
> +	err = notify_change(lowerd, &newattr, NULL);
>  	revert_creds(oldcred);
>  	inode_unlock(loweri);
>  
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team