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: <c179d808-66ee-a4b5-b3f1-91ba336a68ca@kernel.org> Date: Sat, 28 Jan 2023 18:34:07 +0800 From: Chao Yu <chao@...nel.org> To: Yangtao Li <frank.li@...o.com>, jaegeuk@...nel.org Cc: linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org Subject: Re: [PATCH 2/3] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION_V2 ioctl On 2023/1/12 21:35, Yangtao Li wrote: > Added a new F2FS_IOC_SET_COMPRESS_OPTION_V2 ioctl to change file > compression option of a file. > > struct f2fs_comp_option_v2 { > union { > struct { > __u8 algorithm; > __u8 log_cluster_size; > __u16 compress_flag; > }; > struct f2fs_comp_option option; > }; > }; > > struct f2fs_comp_option_v2 option; > > option.algorithm = 2; > option.log_cluster_size = 2; > option.compress_flag = (5 << COMPRESS_LEVEL_OFFSET) | BIT(COMPRESS_CHKSUM); > > ioctl(fd, F2FS_IOC_SET_COMPRESS_OPTION_V2, &option); > > Signed-off-by: Yangtao Li <frank.li@...o.com> > --- > fs/f2fs/f2fs.h | 8 +------- > fs/f2fs/file.c | 41 ++++++++++++++++++++++++++++++++------- > include/uapi/linux/f2fs.h | 21 ++++++++++++++++++++ > 3 files changed, 56 insertions(+), 14 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index f3c5f7740c1a..c2267f44bcf8 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -25,6 +25,7 @@ > #include <linux/quotaops.h> > #include <linux/part_stat.h> > #include <crypto/hash.h> > +#include <uapi/linux/f2fs.h> > > #include <linux/fscrypt.h> > #include <linux/fsverity.h> > @@ -1501,11 +1502,6 @@ enum compress_algorithm_type { > COMPRESS_MAX, > }; > > -enum compress_flag { > - COMPRESS_CHKSUM, > - COMPRESS_MAX_FLAG, > -}; > - > #define COMPRESS_WATERMARK 20 > #define COMPRESS_PERCENT 20 > > @@ -1521,8 +1517,6 @@ struct compress_data { > > #define F2FS_COMPRESSED_PAGE_MAGIC 0xF5F2C000 > > -#define COMPRESS_LEVEL_OFFSET 8 > - > /* compress context */ > struct compress_ctx { > struct inode *inode; /* inode the context belong to */ > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index f5c1b7814954..719706ef0d46 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -25,6 +25,8 @@ > #include <linux/fileattr.h> > #include <linux/fadvise.h> > #include <linux/iomap.h> > +#include <linux/zstd.h> > +#include <linux/lz4.h> > > #include "f2fs.h" > #include "node.h" > @@ -3910,12 +3912,13 @@ static int f2fs_ioc_get_compress_option(struct file *filp, unsigned long arg) > return 0; > } > > -static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) > +static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg, > + unsigned int cmd) > { > struct inode *inode = file_inode(filp); > struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > - struct f2fs_comp_option option; > - int ret = 0; > + struct f2fs_comp_option_v2 option; > + int ret = 0, len; > > if (!f2fs_sb_has_compression(sbi)) > return -EOPNOTSUPP; > @@ -3923,8 +3926,12 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) > if (!(filp->f_mode & FMODE_WRITE)) > return -EBADF; > > - if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg, > - sizeof(option))) > + if (cmd == F2FS_IOC_SET_COMPRESS_OPTION_V2) > + len = sizeof(struct f2fs_comp_option_v2); > + else > + len = sizeof(struct f2fs_comp_option); > + > + if (copy_from_user(&option, (void __user *)arg, len)) > return -EFAULT; > > if (!f2fs_compressed_file(inode) || > @@ -3933,6 +3940,21 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) > option.algorithm >= COMPRESS_MAX) > return -EINVAL; > > + if (cmd == F2FS_IOC_SET_COMPRESS_OPTION_V2) { > + unsigned int level = GET_COMPRESS_LEVEL(option.compress_flag); > + > + switch (option.algorithm) { > + case COMPRESS_LZ4: > + if (level < LZ4HC_MIN_CLEVEL || level > LZ4HC_MAX_CLEVEL) > + return -EINVAL; > + break; level=0 indicates lz4, so it's allowed? > + case COMPRESS_ZSTD: > + if (!level || level > zstd_max_clevel()) > + return -EINVAL; > + break; default: return -EINVAL; > + } > + } > + > file_start_write(filp); > inode_lock(inode); > > @@ -3948,7 +3970,10 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) > > F2FS_I(inode)->i_compress_algorithm = option.algorithm; > F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size; > - F2FS_I(inode)->i_cluster_size = 1 << option.log_cluster_size; > + F2FS_I(inode)->i_cluster_size = BIT(option.log_cluster_size); > + > + if (cmd == F2FS_IOC_SET_COMPRESS_OPTION_V2) > + F2FS_I(inode)->i_compress_flag = option.compress_flag & COMPRESS_OPTION_MASK; > f2fs_mark_inode_dirty_sync(inode, true); > > if (!f2fs_is_compress_backend_ready(inode)) > @@ -4221,7 +4246,9 @@ static long __f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > case F2FS_IOC_GET_COMPRESS_OPTION: > return f2fs_ioc_get_compress_option(filp, arg); > case F2FS_IOC_SET_COMPRESS_OPTION: > - return f2fs_ioc_set_compress_option(filp, arg); > + return f2fs_ioc_set_compress_option(filp, arg, F2FS_IOC_SET_COMPRESS_OPTION); > + case F2FS_IOC_SET_COMPRESS_OPTION_V2: > + return f2fs_ioc_set_compress_option(filp, arg, F2FS_IOC_SET_COMPRESS_OPTION_V2); It needs to adapt f2fs_compat_ioctl() as well. > case F2FS_IOC_DECOMPRESS_FILE: > return f2fs_ioc_decompress_file(filp, arg); > case F2FS_IOC_COMPRESS_FILE: > diff --git a/include/uapi/linux/f2fs.h b/include/uapi/linux/f2fs.h > index 955d440be104..aaf7f55273fb 100644 > --- a/include/uapi/linux/f2fs.h > +++ b/include/uapi/linux/f2fs.h > @@ -43,6 +43,8 @@ > #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_SET_COMPRESS_OPTION_V2 _IOW(F2FS_IOCTL_MAGIC, 26, \ > + struct f2fs_comp_option_v2) > > /* > * should be same as XFS_IOC_GOINGDOWN. > @@ -62,6 +64,15 @@ > #define F2FS_TRIM_FILE_ZEROOUT 0x2 /* zero out */ > #define F2FS_TRIM_FILE_MASK 0x3 > > +/* > + * Flags used by F2FS_IOC_SET_COMPRESS_OPTION_V2 > + */ > +#define COMPRESS_CHKSUM 0x0 /* enable chksum for compress file */ > +#define COMPRESS_LEVEL_OFFSET 8 > +#define COMPRESS_LEVEL_MASK GENMASK(15, COMPRESS_LEVEL_OFFSET) > +#define COMPRESS_OPTION_MASK (COMPRESS_LEVEL_MASK | BIT(COMPRESS_CHKSUM)) > +#define GET_COMPRESS_LEVEL(x) (((x) & COMPRESS_LEVEL_MASK) >> COMPRESS_LEVEL_OFFSET) > + > struct f2fs_gc_range { > __u32 sync; > __u64 start; > @@ -96,4 +107,14 @@ struct f2fs_comp_option { > __u8 log_cluster_size; > }; > > +struct f2fs_comp_option_v2 { > + union { > + struct { > + __u8 algorithm; > + __u8 log_cluster_size; > + __u16 compress_flag; > + }; > + struct f2fs_comp_option option; > + }; > +}; It looks using union may simply the implementation, but readability of new structure becomes worse, since this structure will be exposed to user as an uapi interface, I guess we'd better to consider more about its readability, thought? Thanks, > #endif /* _UAPI_LINUX_F2FS_H */
Powered by blists - more mailing lists