linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE,GARBAGE_COLLECT}_RANGE
@ 2020-11-04  2:19 Chao Yu
  2020-11-04  2:29 ` [f2fs-dev] [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE, GARBAGE_COLLECT}_RANGE Eric Biggers
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2020-11-04  2:19 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Eric reported a ioctl bug in below link:

https://lore.kernel.org/linux-f2fs-devel/20201103032234.GB2875@sol.localdomain/

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 betweeen 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

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---

Jaegeuk, Eric,

I have no 32-bit machine now, so I don't run any test on this patch,
please take a look at this RFC patch first.

 fs/f2fs/file.c            | 45 +++++++++++++++++++++++++++++++++++++--
 include/uapi/linux/f2fs.h | 25 ++++++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 52417a2e3f4f..2e0a5469745a 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4236,6 +4236,45 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 }
 
 #ifdef CONFIG_COMPAT
+static int f2fs_compat_ioc_gc_range(struct file *file, unsigned long arg)
+{
+	struct compat_f2fs_gc_range __user *urange;
+	struct f2fs_gc_range range;
+	int err;
+
+	urange = compat_ptr(arg);
+	err = get_user(range.sync, &urange->sync);
+	err |= get_user(range.start, &urange->start);
+	err |= get_user(range.len, &urange->len);
+	if (err)
+		return -EFAULT;
+	if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(file)))))
+		return -EIO;
+	if (!f2fs_is_checkpoint_ready(F2FS_I_SB(file_inode(file))))
+		return -ENOSPC;
+	return f2fs_ioc_gc_range(file, (unsigned long)&range);
+}
+
+static int f2fs_compat_ioc_move_range(struct file *file, unsigned long arg)
+{
+	struct compat_f2fs_move_range __user *urange;
+	struct f2fs_move_range range;
+	int err;
+
+	urange = compat_ptr(arg);
+	err = get_user(range.dst_fd, &urange->dst_fd);
+	err |= get_user(range.pos_in, &urange->pos_in);
+	err |= get_user(range.pos_out, &urange->pos_out);
+	err |= get_user(range.len, &urange->len);
+	if (err)
+		return -EFAULT;
+	if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(file)))))
+		return -EIO;
+	if (!f2fs_is_checkpoint_ready(F2FS_I_SB(file_inode(file))))
+		return -ENOSPC;
+	return f2fs_ioc_move_range(file, (unsigned long)&range);
+}
+
 long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	switch (cmd) {
@@ -4248,6 +4287,10 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case FS_IOC32_GETVERSION:
 		cmd = FS_IOC_GETVERSION;
 		break;
+	case F2FS_IOC32_GARBAGE_COLLECT_RANGE:
+		return f2fs_compat_ioc_gc_range(file, arg);
+	case F2FS_IOC32_MOVE_RANGE:
+		return f2fs_compat_ioc_move_range(file, arg);
 	case F2FS_IOC_START_ATOMIC_WRITE:
 	case F2FS_IOC_COMMIT_ATOMIC_WRITE:
 	case F2FS_IOC_START_VOLATILE_WRITE:
@@ -4265,10 +4308,8 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
 	case FS_IOC_GET_ENCRYPTION_NONCE:
 	case F2FS_IOC_GARBAGE_COLLECT:
-	case F2FS_IOC_GARBAGE_COLLECT_RANGE:
 	case F2FS_IOC_WRITE_CHECKPOINT:
 	case F2FS_IOC_DEFRAGMENT:
-	case F2FS_IOC_MOVE_RANGE:
 	case F2FS_IOC_FLUSH_DEVICE:
 	case F2FS_IOC_GET_FEATURES:
 	case FS_IOC_FSGETXATTR:
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 */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE, GARBAGE_COLLECT}_RANGE
  2020-11-04  2:19 [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE,GARBAGE_COLLECT}_RANGE Chao Yu
@ 2020-11-04  2:29 ` Eric Biggers
  2020-11-04  2:52   ` Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2020-11-04  2:29 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel

On Wed, Nov 04, 2020 at 10:19:06AM +0800, Chao Yu wrote:
> Eric reported a ioctl bug in below link:
> 
> https://lore.kernel.org/linux-f2fs-devel/20201103032234.GB2875@sol.localdomain/
> 
> 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 betweeen 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
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> 
> Jaegeuk, Eric,
> 
> I have no 32-bit machine now, so I don't run any test on this patch,
> please take a look at this RFC patch first.

You can test this by running a 32-bit binary on a machine with a 64-bit kernel.
E.g. on x86_64, compile a binary with 'gcc -m32'.

>  #ifdef CONFIG_COMPAT
> +static int f2fs_compat_ioc_gc_range(struct file *file, unsigned long arg)
> +{
> +	struct compat_f2fs_gc_range __user *urange;
> +	struct f2fs_gc_range range;
> +	int err;
> +
> +	urange = compat_ptr(arg);
> +	err = get_user(range.sync, &urange->sync);
> +	err |= get_user(range.start, &urange->start);
> +	err |= get_user(range.len, &urange->len);
> +	if (err)
> +		return -EFAULT;
> +	if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(file)))))
> +		return -EIO;
> +	if (!f2fs_is_checkpoint_ready(F2FS_I_SB(file_inode(file))))
> +		return -ENOSPC;
> +	return f2fs_ioc_gc_range(file, (unsigned long)&range);
> +}

This won't work because f2fs_ioc_gc_range() expects a user pointer.  You'll need
to make the native and compat versions do the copy from user separately, and
have them call a helper function that takes a pointer to the argument in kernel
memory.

- Eric

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE, GARBAGE_COLLECT}_RANGE
  2020-11-04  2:29 ` [f2fs-dev] [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE, GARBAGE_COLLECT}_RANGE Eric Biggers
@ 2020-11-04  2:52   ` Chao Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2020-11-04  2:52 UTC (permalink / raw)
  To: Eric Biggers; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel

On 2020/11/4 10:29, Eric Biggers wrote:
> On Wed, Nov 04, 2020 at 10:19:06AM +0800, Chao Yu wrote:
>> Eric reported a ioctl bug in below link:
>>
>> https://lore.kernel.org/linux-f2fs-devel/20201103032234.GB2875@sol.localdomain/
>>
>> 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 betweeen 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
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>
>> Jaegeuk, Eric,
>>
>> I have no 32-bit machine now, so I don't run any test on this patch,
>> please take a look at this RFC patch first.
> 
> You can test this by running a 32-bit binary on a machine with a 64-bit kernel.
> E.g. on x86_64, compile a binary with 'gcc -m32'.

Ah, thanks for providing the option, I can try that.

> 
>>   #ifdef CONFIG_COMPAT
>> +static int f2fs_compat_ioc_gc_range(struct file *file, unsigned long arg)
>> +{
>> +	struct compat_f2fs_gc_range __user *urange;
>> +	struct f2fs_gc_range range;
>> +	int err;
>> +
>> +	urange = compat_ptr(arg);
>> +	err = get_user(range.sync, &urange->sync);
>> +	err |= get_user(range.start, &urange->start);
>> +	err |= get_user(range.len, &urange->len);
>> +	if (err)
>> +		return -EFAULT;
>> +	if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(file)))))
>> +		return -EIO;
>> +	if (!f2fs_is_checkpoint_ready(F2FS_I_SB(file_inode(file))))
>> +		return -ENOSPC;
>> +	return f2fs_ioc_gc_range(file, (unsigned long)&range);
>> +}
> 
> This won't work because f2fs_ioc_gc_range() expects a user pointer.  You'll need
> to make the native and compat versions do the copy from user separately, and
> have them call a helper function that takes a pointer to the argument in kernel
> memory.

Yes, I need to introduce function like __f2fs_ioc_gc_range() to accepted pointer
parameter which points kernel memory.

Will do test and resend v2 soon.

Thanks,

> 
> - Eric
> .
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-11-04  2:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-04  2:19 [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE,GARBAGE_COLLECT}_RANGE Chao Yu
2020-11-04  2:29 ` [f2fs-dev] [PATCH RFC] f2fs: fix compat F2FS_IOC_{MOVE, GARBAGE_COLLECT}_RANGE Eric Biggers
2020-11-04  2:52   ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).