Re: [PATCH] f2fs: require CAP_SYS_ADMIN for fallocate on pinned files

[Chao Yu]

On 11/8/26 15:53, Hungyu Lin wrote:
> fallocate() on a pinned file allocates blocks without zeroing them, so
> reading the file returns whatever the blocks held before.
> Documentation/filesystems/f2fs.rst describes this as "zero or random
> data". Commit cad3836f9eb2 ("f2fs: allocate blocks for pinned file")
> added this while F2FS_IOC_SET_PIN_FILE required CAP_SYS_ADMIN, but
> commit b13bdf03bb80 ("f2fs: allow all the users to pin a file") then let
> any user pin a file. Since then any user can create a file, pin it,
> fallocate() it and read back data of files that other users have
> deleted.

Android should be safe, as on-disk data from user is encrypted, it will be
decrypted only in memory w/ user's crypto key, there is no way to read the
plain data from disk directly.

> 
> Require CAP_SYS_ADMIN for fallocate() on a pinned file. The documented
> use of these blocks needs write access to the block device anyway, and
> Android's gsid, which creates pinned image files through libfiemap, has
> CAP_SYS_ADMIN. Users without CAP_SYS_ADMIN can still pin files, but can
> no longer preallocate blocks for them. fallocate() on files that are not
> pinned is not affected.

Or maybe we can call f2fs_secure_erase() on the blkaddr range after
preallocation and before return from fallocate().

> 
> Fixes: b13bdf03bb80 ("f2fs: allow all the users to pin a file")

Jaegeuk, do we suffer any permission issue before above commit?

Thanks,

> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hungyu Lin <dennylin0707@xxxxxxxxx>
> ---
>  fs/f2fs/file.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index e2c0f5083313..d1b52b41a1eb 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1973,6 +1973,9 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
>  	block_t expanded = 0;
>  	int err;
>  
> +	if (f2fs_is_pinned_file(inode) && !capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
>  	err = inode_newsize_ok(inode, (len + offset));
>  	if (err)
>  		return err;