On Sun, 19 Jan 2025 at 16:00, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Current efivarfs uses simple_setattr which allows the setting of any
> size in the inode cache. This is wrong because a zero size file is
> used to indicate an "uncommitted" variable, so by simple means of
> truncating the file (as root) any variable may be turned to look like
> it's uncommitted. Fix by adding an efivarfs_setattr routine which
> does not allow updating of the cached inode size (which now only comes
> from the underlying variable).
>
> Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> ---
> fs/efivarfs/inode.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
> index ec23da8405ff..a4a6587ecd2e 100644
> --- a/fs/efivarfs/inode.c
> +++ b/fs/efivarfs/inode.c
> @@ -187,7 +187,24 @@ efivarfs_fileattr_set(struct mnt_idmap *idmap,
> return 0;
> }
>
> +/* copy of simple_setattr except that it doesn't do i_size updates */
> +static int efivarfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> + struct iattr *iattr)
> +{
> + struct inode *inode = d_inode(dentry);
> + int error;
> +
> + error = setattr_prepare(idmap, dentry, iattr);
> + if (error)
> + return error;
> +
> + setattr_copy(idmap, inode, iattr);
> + mark_inode_dirty(inode);
> + return 0;
> +}
> +
> static const struct inode_operations efivarfs_file_inode_operations = {
> .fileattr_get = efivarfs_fileattr_get,
> .fileattr_set = efivarfs_fileattr_set,
> + .setattr = efivarfs_setattr,
> };
Is it sufficient to just ignore inode size changes? Should we complain
about this instead?
