This patch is marked 2/2, but it seems you sent it out on its own. Patch series are supposed to be resend in full; otherwise people can see just one patch and have no context.
On Thu, Nov 05, 2020 at 09:09:34AM +0800, Chao Yu wrote: > Eric reported a ioctl bug in below link: > > https://lore.kernel.org/linux-f2fs-devel/[email protected]/ > > That said, on some 32-bit architectures, u64 has only 32-bit alignment, > notably i386 and x86_32, so that size of struct f2fs_gc_range compiled > in x86_32 is 20 bytes, however the size in x86_64 is 24 bytes, binary > compiled in x86_32 can not call F2FS_IOC_GARBAGE_COLLECT_RANGE successfully > due to mismatched value of ioctl command in between binary and f2fs > module, similarly, F2FS_IOC_MOVE_RANGE will fail too. > > In this patch we introduce two ioctls for compatibility of above special > 32-bit binary: > - F2FS_IOC32_GARBAGE_COLLECT_RANGE > - F2FS_IOC32_MOVE_RANGE > It would be good to add a proper reported-by line, otherwise it's not clear who "Eric" is (there are lots of Erics): Reported-by: Eric Biggers <[email protected]> > +static int __f2fs_ioc_gc_range(struct file *filp, struct f2fs_gc_range > *range) > { > - struct inode *inode = file_inode(filp); > - struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > - struct f2fs_gc_range range; > + struct f2fs_sb_info *sbi = F2FS_I_SB(file_inode(filp)); > u64 end; > int ret; > > + if (unlikely(f2fs_cp_error(sbi))) > + return -EIO; > + if (!f2fs_is_checkpoint_ready(sbi)) > + return -ENOSPC; These two checkpoint-related checks weren't present in the original version. Is that intentional? > +static int __f2fs_ioc_move_range(struct file *filp, > + struct f2fs_move_range *range) > { > - struct f2fs_move_range range; > + struct f2fs_sb_info *sbi = F2FS_I_SB(file_inode(filp)); > struct fd dst; > int err; > > + if (unlikely(f2fs_cp_error(sbi))) > + return -EIO; > + if (!f2fs_is_checkpoint_ready(sbi)) > + return -ENOSPC; > + Likewise here. > diff --git a/include/uapi/linux/f2fs.h b/include/uapi/linux/f2fs.h > index f00199a2e38b..8c14e88a9645 100644 > --- a/include/uapi/linux/f2fs.h > +++ b/include/uapi/linux/f2fs.h > @@ -5,6 +5,10 @@ > #include <linux/types.h> > #include <linux/ioctl.h> > > +#ifdef __KERNEL__ > +#include <linux/compat.h> > +#endif > + > /* > * f2fs-specific ioctl commands > */ > @@ -65,6 +69,16 @@ struct f2fs_gc_range { > __u64 len; > }; > > +#if defined(__KERNEL__) && defined(CONFIG_COMPAT) > +struct compat_f2fs_gc_range { > + u32 sync; > + compat_u64 start; > + compat_u64 len; > +}; > +#define F2FS_IOC32_GARBAGE_COLLECT_RANGE _IOW(F2FS_IOCTL_MAGIC, 11,\ > + struct compat_f2fs_gc_range) > +#endif > + > struct f2fs_defragment { > __u64 start; > __u64 len; > @@ -77,6 +91,17 @@ struct f2fs_move_range { > __u64 len; /* size to move */ > }; > > +#if defined(__KERNEL__) && defined(CONFIG_COMPAT) > +struct compat_f2fs_move_range { > + u32 dst_fd; > + compat_u64 pos_in; > + compat_u64 pos_out; > + compat_u64 len; > +}; > +#define F2FS_IOC32_MOVE_RANGE _IOWR(F2FS_IOCTL_MAGIC, 9, > \ > + struct compat_f2fs_move_range) > +#endif > + > struct f2fs_flush_device { > __u32 dev_num; /* device number to flush */ > __u32 segments; /* # of segments to flush */ > -- Did you consider instead putting these compat definitions in an internal kernel header, or even just in the .c file, to avoid cluttering up the UAPI header? - Eric _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
