| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <66107ad2-1c1e-7e25-a8f5-167f4d06525c@oppo.com>
Date: Tue, 13 Jun 2023 09:53:06 +0800
From: Sheng Yong <shengyong@...o.com>
To: Jaegeuk Kim <jaegeuk@...nel.org>
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, ebiggers@...nel.org
Subject: Re: [PATCH v4 5/6] f2fs: add f2fs_ioc_[get|set]_extra_attr
On 2023/6/12 23:47, Jaegeuk Kim wrote:
> On 06/12, Sheng Yong wrote:
>> This patch introduces two ioctls:
>> * f2fs_ioc_get_extra_attr
>> * f2fs_ioc_set_extra_attr
>> to get or modify values in f2fs_inode's extra attribute area.
>
> What'd be the main purpose of this new ioctl? Use-cases or examples?
Hi, Jaegeuk,
When a new feature is added, f2fs_inode extra attributes needs to be
extended sometimes to save status or control values, say compression,
inode chksum, inode crtime.
If some of them could be read or modified during runtime, a new ioctl
is required in most cases, like F2FS_IOC_GET_COMPRESS_BLOCKS,
F2FS_IOC_GET_COMPRESS_OPTION...
And Yangtao [1] and I [2] also tried to add new ioctls to allow getting
and settingcompress level and flags.
If new features are added in the future, we may get a verty long and
not-easy-to-extend ioctl list.
So these 2 ioctls could help manage all extra attributes:
* if a new extra attribute is added, we only need to add a new field type,
and corresponding get/set functions
* attr_size can indicate which version of attribute is used
* besides, ioc_set_extra_attr(i_inline_xattr_size) gives us a chance to
adjust inline xattr size for an empty file
thanks,
shengyong
Links:
[1] https://lore.kernel.org/linux-f2fs-devel/20230112133503.16802-1-frank.li@vivo.com/
[2] https://sourceforge.net/p/linux-f2fs/mailman/linux-f2fs-devel/?viewmonth=202303
>
>>
>> The argument of these two ioctls is `struct f2fs_extra_attr', which has
>> three members:
>> * field: indicates which field in extra attribute area is handled
>> * attr: value or userspace pointer
>> * attr_size: size of `attr'
>>
>> The `field' member could help extend functionality of these two ioctls
>> without modify or add new interfaces, if more fields are added into
>> extra attributes ares in the feture.
>>
>> Signed-off-by: Sheng Yong <shengyong@...o.com>
>> ---
>> fs/f2fs/file.c | 205 ++++++++++++++++++++++++++++++++++++++
>> include/uapi/linux/f2fs.h | 25 +++++
>> 2 files changed, 230 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index f8aa842b5d233..39d04f8f0bb6b 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -4179,6 +4179,207 @@ static int f2fs_ioc_compress_file(struct file *filp)
>> return ret;
>> }
>>
>> +static bool extra_attr_fits_in_inode(struct inode *inode, int field)
>> +{
>> + struct f2fs_inode_info *fi = F2FS_I(inode);
>> + struct f2fs_inode *ri;
>> +
>> + switch (field) {
>> + case F2FS_EXTRA_ATTR_TOTAL_SIZE:
>> + case F2FS_EXTRA_ATTR_ISIZE:
>> + case F2FS_EXTRA_ATTR_INLINE_XATTR_SIZE:
>> + return true;
>> + case F2FS_EXTRA_ATTR_PROJID:
>> + if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_projid))
>> + return false;
>> + return true;
>> + case F2FS_EXTRA_ATTR_INODE_CHKSUM:
>> + if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_inode_checksum))
>> + return false;
>> + return true;
>> + case F2FS_EXTRA_ATTR_CRTIME:
>> + if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_crtime))
>> + return false;
>> + return true;
>> + case F2FS_EXTRA_ATTR_COMPR_BLOCKS:
>> + case F2FS_EXTRA_ATTR_COMPR_OPTION:
>> + if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_compr_blocks))
>> + return false;
>> + return true;
>> + default:
>> + BUG_ON(1);
>> + return false;
>> + }
>> +}
>> +
>> +static int f2fs_ioc_get_extra_attr(struct file *filp, unsigned long arg)
>> +{
>> + struct inode *inode = file_inode(filp);
>> + struct f2fs_inode_info *fi = F2FS_I(inode);
>> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> + struct f2fs_extra_attr attr;
>> + u32 chksum;
>> + int ret = 0;
>> +
>> + if (!f2fs_has_extra_attr(inode))
>> + return -EOPNOTSUPP;
>> +
>> + if (copy_from_user(&attr, (void __user *)arg, sizeof(attr)))
>> + return -EFAULT;
>> +
>> + if (attr.field >= F2FS_EXTRA_ATTR_MAX)
>> + return -EINVAL;
>> +
>> + if (!extra_attr_fits_in_inode(inode, attr.field))
>> + return -EOPNOTSUPP;
>> +
>> + switch (attr.field) {
>> + case F2FS_EXTRA_ATTR_TOTAL_SIZE:
>> + attr.attr = F2FS_TOTAL_EXTRA_ATTR_SIZE;
>> + break;
>> + case F2FS_EXTRA_ATTR_ISIZE:
>> + attr.attr = fi->i_extra_isize;
>> + break;
>> + case F2FS_EXTRA_ATTR_INLINE_XATTR_SIZE:
>> + if (!f2fs_has_inline_xattr(inode))
>> + return -EOPNOTSUPP;
>> + attr.attr = get_inline_xattr_addrs(inode);
>> + break;
>> + case F2FS_EXTRA_ATTR_PROJID:
>> + if (!f2fs_sb_has_project_quota(F2FS_I_SB(inode)))
>> + return -EOPNOTSUPP;
>> + attr.attr = from_kprojid(&init_user_ns, fi->i_projid);
>> + break;
>> + case F2FS_EXTRA_ATTR_INODE_CHKSUM:
>> + ret = f2fs_inode_chksum_get(sbi, inode, &chksum);
>> + if (ret)
>> + return ret;
>> + attr.attr = chksum;
>> + break;
>> + case F2FS_EXTRA_ATTR_CRTIME:
>> + if (!f2fs_sb_has_inode_crtime(sbi))
>> + return -EOPNOTSUPP;
>> + if (attr.attr_size == sizeof(struct timespec64)) {
>> + if (put_timespec64(&fi->i_crtime,
>> + (void __user *)(uintptr_t)attr.attr))
>> + return -EFAULT;
>> + } else if (attr.attr_size == sizeof(struct old_timespec32)) {
>> + if (put_old_timespec32(&fi->i_crtime,
>> + (void __user *)(uintptr_t)attr.attr))
>> + return -EFAULT;
>> + } else {
>> + return -EINVAL;
>> + }
>> + break;
>> + case F2FS_EXTRA_ATTR_COMPR_BLOCKS:
>> + if (attr.attr_size != sizeof(__u64))
>> + return -EINVAL;
>> + ret = f2fs_get_compress_blocks(inode, &attr.attr);
>> + break;
>> + case F2FS_EXTRA_ATTR_COMPR_OPTION:
>> + ret = f2fs_ioc_get_compress_option(filp, attr.attr);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (copy_to_user((void __user *)arg, &attr, sizeof(attr)))
>> + return -EFAULT;
>> +
>> + return 0;
>> +}
>> +
>> +static int f2fs_ioc_set_extra_attr(struct file *filp, unsigned long arg)
>> +{
>> + struct inode *inode = file_inode(filp);
>> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> + struct f2fs_extra_attr attr;
>> + struct page *ipage;
>> + void *inline_addr;
>> + int ret;
>> +
>> + if (!f2fs_has_extra_attr(inode))
>> + return -EOPNOTSUPP;
>> +
>> + if (copy_from_user(&attr, (void __user *)arg, sizeof(attr)))
>> + return -EFAULT;
>> +
>> + if (attr.field >= F2FS_EXTRA_ATTR_MAX)
>> + return -EINVAL;
>> +
>> + if (!extra_attr_fits_in_inode(inode, attr.field))
>> + return -EOPNOTSUPP;
>> +
>> + switch (attr.field) {
>> + case F2FS_EXTRA_ATTR_TOTAL_SIZE:
>> + case F2FS_EXTRA_ATTR_ISIZE:
>> + case F2FS_EXTRA_ATTR_PROJID:
>> + case F2FS_EXTRA_ATTR_INODE_CHKSUM:
>> + case F2FS_EXTRA_ATTR_CRTIME:
>> + case F2FS_EXTRA_ATTR_COMPR_BLOCKS:
>> + /* read only attribtues */
>> + return -EOPNOTSUPP;
>> + case F2FS_EXTRA_ATTR_INLINE_XATTR_SIZE:
>> + if (!f2fs_sb_has_flexible_inline_xattr(sbi) ||
>> + !f2fs_has_inline_xattr(inode))
>> + return -EOPNOTSUPP;
>> + if (attr.attr < MIN_INLINE_XATTR_SIZE ||
>> + attr.attr > MAX_INLINE_XATTR_SIZE)
>> + return -EINVAL;
>> + inode_lock(inode);
>> + f2fs_lock_op(sbi);
>> + f2fs_down_write(&F2FS_I(inode)->i_xattr_sem);
>> + if (i_size_read(inode) || F2FS_I(inode)->i_xattr_nid) {
>> + /*
>> + * it is not allowed to set this field if the inode
>> + * has data or xattr node
>> + */
>> + ret = -EFBIG;
>> + goto xattr_out_unlock;
>> + }
>> + ipage = f2fs_get_node_page(sbi, inode->i_ino);
>> + if (IS_ERR(ipage)) {
>> + ret = PTR_ERR(ipage);
>> + goto xattr_out_unlock;
>> + }
>> + inline_addr = inline_xattr_addr(inode, ipage);
>> + if (!IS_XATTR_LAST_ENTRY(XATTR_FIRST_ENTRY(inline_addr))) {
>> + ret = -EFBIG;
>> + } else {
>> + struct f2fs_xattr_header *hdr;
>> + struct f2fs_xattr_entry *ent;
>> +
>> + F2FS_I(inode)->i_inline_xattr_size = (int)attr.attr;
>> + inline_addr = inline_xattr_addr(inode, ipage);
>> + hdr = XATTR_HDR(inline_addr);
>> + ent = XATTR_FIRST_ENTRY(inline_addr);
>> + hdr->h_magic = cpu_to_le32(F2FS_XATTR_MAGIC);
>> + hdr->h_refcount = cpu_to_le32(1);
>> + memset(ent, 0, attr.attr - sizeof(*hdr));
>> + set_page_dirty(ipage);
>> + ret = 0;
>> + }
>> + f2fs_put_page(ipage, 1);
>> +xattr_out_unlock:
>> + f2fs_up_write(&F2FS_I(inode)->i_xattr_sem);
>> + f2fs_unlock_op(sbi);
>> + inode_unlock(inode);
>> + if (!ret)
>> + f2fs_balance_fs(sbi, true);
>> + break;
>> + case F2FS_EXTRA_ATTR_COMPR_OPTION:
>> + ret = f2fs_ioc_set_compress_option(filp, attr.attr);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static long __f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> {
>> switch (cmd) {
>> @@ -4265,6 +4466,10 @@ static long __f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> return f2fs_ioc_decompress_file(filp);
>> case F2FS_IOC_COMPRESS_FILE:
>> return f2fs_ioc_compress_file(filp);
>> + case F2FS_IOC_GET_EXTRA_ATTR:
>> + return f2fs_ioc_get_extra_attr(filp, arg);
>> + case F2FS_IOC_SET_EXTRA_ATTR:
>> + return f2fs_ioc_set_extra_attr(filp, arg);
>> default:
>> return -ENOTTY;
>> }
>> diff --git a/include/uapi/linux/f2fs.h b/include/uapi/linux/f2fs.h
>> index 955d440be1046..2b53e90421bfc 100644
>> --- a/include/uapi/linux/f2fs.h
>> +++ b/include/uapi/linux/f2fs.h
>> @@ -43,6 +43,10 @@
>> #define F2FS_IOC_DECOMPRESS_FILE _IO(F2FS_IOCTL_MAGIC, 23)
>> #define F2FS_IOC_COMPRESS_FILE _IO(F2FS_IOCTL_MAGIC, 24)
>> #define F2FS_IOC_START_ATOMIC_REPLACE _IO(F2FS_IOCTL_MAGIC, 25)
>> +#define F2FS_IOC_GET_EXTRA_ATTR _IOR(F2FS_IOCTL_MAGIC, 26, \
>> + struct f2fs_extra_attr)
>> +#define F2FS_IOC_SET_EXTRA_ATTR _IOW(F2FS_IOCTL_MAGIC, 27, \
>> + struct f2fs_extra_attr)
>>
>> /*
>> * should be same as XFS_IOC_GOINGDOWN.
>> @@ -96,4 +100,25 @@ struct f2fs_comp_option {
>> __u8 log_cluster_size;
>> };
>>
>> +enum {
>> + F2FS_EXTRA_ATTR_TOTAL_SIZE, /* ro, size of extra attr area */
>> + F2FS_EXTRA_ATTR_ISIZE, /* ro, i_extra_isize */
>> + F2FS_EXTRA_ATTR_INLINE_XATTR_SIZE, /* rw, i_inline_xattr_size */
>> + F2FS_EXTRA_ATTR_PROJID, /* ro, i_projid */
>> + F2FS_EXTRA_ATTR_INODE_CHKSUM, /* ro, i_inode_chksum */
>> + F2FS_EXTRA_ATTR_CRTIME, /* ro, i_crtime, i_crtime_nsec */
>> + F2FS_EXTRA_ATTR_COMPR_BLOCKS, /* ro, i_compr_blocks */
>> + F2FS_EXTRA_ATTR_COMPR_OPTION, /* rw, i_compress_algorithm,
>> + * i_log_cluster_size
>> + */
>> + F2FS_EXTRA_ATTR_MAX,
>> +};
>> +
>> +struct f2fs_extra_attr {
>> + __u8 field; /* F2FS_EXTRA_ATTR_* */
>> + __u8 rsvd1;
>> + __u16 attr_size; /* size of @attr */
>> + __u32 rsvd2;
>> + __u64 attr; /* attr value or pointer */
>> +};
>> #endif /* _UAPI_LINUX_F2FS_H */
>> --
>> 2.40.1
Powered by blists - more mailing lists