Re: [patch 12/12] vfs: allow file truncations when both suid and write permissions set

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

akpm@linux-foundation.org writes:

> diff -puN fs/open.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set fs/open.c
> --- a/fs/open.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set
> +++ a/fs/open.c
> @@ -213,11 +213,15 @@ int do_truncate(struct dentry *dentry, l
>  		newattrs.ia_valid |= ATTR_FILE;
>  	}
>  
> +	mutex_lock(&dentry->d_inode->i_mutex);
>  	/* Remove suid/sgid on truncate too */
> -	newattrs.ia_valid |= should_remove_suid(dentry);
> +	err = dentry_remove_suid(dentry);
> +	if (err)
> +		goto unlock;

Can't we use ATTR_FORCE for this? Because this calls notify_change()
twice, and I guess this removes s[ug]id even if vmtruncate() (or in
future ->truncate() may return error) or something returned error.

I think it would not be good behavior.

Thanks.

> -	mutex_lock(&dentry->d_inode->i_mutex);
>  	err = notify_change(dentry, &newattrs);
> +
> + unlock:
>  	mutex_unlock(&dentry->d_inode->i_mutex);
>  	return err;
>  }
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>