[rh7] fs: Resurrect generic_segment_checks()

Submitted by Konstantin Khorenko on Dec. 29, 2020, 1:40 p.m.

Details

Message ID 20201229134013.5144-1-khorenko@virtuozzo.com
State New
Series "fs: Resurrect generic_segment_checks()"
Headers show

Commit Message

Konstantin Khorenko Dec. 29, 2020, 1:40 p.m.
This is a partial revert of a9ca9d715754 ("kill generic_segment_checks()").

The patch resurrects the function, but does not restore any call for it.

Why this functiona is needed then?
For zfs.

VZ kernel does not have file_operations::read_iter()/write_iter()
compatible with mainstream version thus zfs external module
tries live without iters and assumes generic_segment_checks() exists.

So keep the generic_segment_checks() specially for zfs.

https://bugs.openvz.org/browse/OVZ-7243

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 include/linux/fs.h |  2 ++
 mm/filemap.c       | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Patch hide | download patch | download mbox

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8398ec202a42..aee8adfc1252 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3255,6 +3255,8 @@  extern ssize_t generic_file_buffered_write_iter(struct kiocb *, struct iov_iter
 		loff_t, loff_t *, ssize_t);
 extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
 extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
+extern int generic_segment_checks(const struct iovec *iov,
+		unsigned long *nr_segs, size_t *count, int access_flags);
 
 /* fs/block_dev.c */
 extern ssize_t blkdev_aio_read(struct kiocb *, const struct iovec *,
diff --git a/mm/filemap.c b/mm/filemap.c
index 950d92b6059b..585c57ef1e0a 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1981,6 +1981,45 @@  int file_read_actor(read_descriptor_t *desc, struct page *page,
 	return size;
 }
 
+/*
+ * Performs necessary checks before doing a write
+ * @iov:	io vector request
+ * @nr_segs:	number of segments in the iovec
+ * @count:	number of bytes to write
+ * @access_flags: type of access: %VERIFY_READ or %VERIFY_WRITE
+ *
+ * Adjust number of segments and amount of bytes to write (nr_segs should be
+ * properly initialized first). Returns appropriate error code that caller
+ * should return or zero in case that write should be allowed.
+ */
+int generic_segment_checks(const struct iovec *iov,
+			unsigned long *nr_segs, size_t *count, int access_flags)
+{
+	unsigned long   seg;
+	size_t cnt = 0;
+	for (seg = 0; seg < *nr_segs; seg++) {
+		const struct iovec *iv = &iov[seg];
+
+		/*
+		 * If any segment has a negative length, or the cumulative
+		 * length ever wraps negative then return -EINVAL.
+		 */
+		cnt += iv->iov_len;
+		if (unlikely((ssize_t)(cnt|iv->iov_len) < 0))
+			return -EINVAL;
+		if (access_ok(access_flags, iv->iov_base, iv->iov_len))
+			continue;
+		if (seg == 0)
+			return -EFAULT;
+		*nr_segs = seg;
+		cnt -= iv->iov_len;	/* This segment is no good */
+		break;
+	}
+	*count = cnt;
+	return 0;
+}
+EXPORT_SYMBOL(generic_segment_checks);
+
 static int file_read_iter_actor(read_descriptor_t *desc, struct page *page,
 				unsigned long offset, unsigned long size)
 {

Comments

Vasily Averin Dec. 30, 2020, 5:32 a.m.
2 questions:
1) Is it used in our kernel?
if not -- I afraid it will not be compiled
if so -- I doubt it should be exported.
By another words:
are you sure "EXPORT_SYMBOL(generic_segment_checks);" line is required?

2) your previous patch-set "fs/iov_iter: Fix ZFS kernel module compilation" is still required too?

Thank you,
	Vasily Averin

On 12/29/20 4:40 PM, Konstantin Khorenko wrote:
> This is a partial revert of a9ca9d715754 ("kill generic_segment_checks()").
> 
> The patch resurrects the function, but does not restore any call for it.
> 
> Why this functiona is needed then?
> For zfs.
> 
> VZ kernel does not have file_operations::read_iter()/write_iter()
> compatible with mainstream version thus zfs external module
> tries live without iters and assumes generic_segment_checks() exists.
> 
> So keep the generic_segment_checks() specially for zfs.
> 
> https://bugs.openvz.org/browse/OVZ-7243
> 
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  include/linux/fs.h |  2 ++
>  mm/filemap.c       | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8398ec202a42..aee8adfc1252 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3255,6 +3255,8 @@ extern ssize_t generic_file_buffered_write_iter(struct kiocb *, struct iov_iter
>  		loff_t, loff_t *, ssize_t);
>  extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
>  extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
> +extern int generic_segment_checks(const struct iovec *iov,
> +		unsigned long *nr_segs, size_t *count, int access_flags);
>  
>  /* fs/block_dev.c */
>  extern ssize_t blkdev_aio_read(struct kiocb *, const struct iovec *,
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 950d92b6059b..585c57ef1e0a 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1981,6 +1981,45 @@ int file_read_actor(read_descriptor_t *desc, struct page *page,
>  	return size;
>  }
>  
> +/*
> + * Performs necessary checks before doing a write
> + * @iov:	io vector request
> + * @nr_segs:	number of segments in the iovec
> + * @count:	number of bytes to write
> + * @access_flags: type of access: %VERIFY_READ or %VERIFY_WRITE
> + *
> + * Adjust number of segments and amount of bytes to write (nr_segs should be
> + * properly initialized first). Returns appropriate error code that caller
> + * should return or zero in case that write should be allowed.
> + */
> +int generic_segment_checks(const struct iovec *iov,
> +			unsigned long *nr_segs, size_t *count, int access_flags)
> +{
> +	unsigned long   seg;
> +	size_t cnt = 0;
> +	for (seg = 0; seg < *nr_segs; seg++) {
> +		const struct iovec *iv = &iov[seg];
> +
> +		/*
> +		 * If any segment has a negative length, or the cumulative
> +		 * length ever wraps negative then return -EINVAL.
> +		 */
> +		cnt += iv->iov_len;
> +		if (unlikely((ssize_t)(cnt|iv->iov_len) < 0))
> +			return -EINVAL;
> +		if (access_ok(access_flags, iv->iov_base, iv->iov_len))
> +			continue;
> +		if (seg == 0)
> +			return -EFAULT;
> +		*nr_segs = seg;
> +		cnt -= iv->iov_len;	/* This segment is no good */
> +		break;
> +	}
> +	*count = cnt;
> +	return 0;
> +}
> +EXPORT_SYMBOL(generic_segment_checks);
> +
>  static int file_read_iter_actor(read_descriptor_t *desc, struct page *page,
>  				unsigned long offset, unsigned long size)
>  {
>
Konstantin Khorenko Dec. 30, 2020, 8:13 a.m.
On 12/30/2020 08:32 AM, Vasily Averin wrote:
> 2 questions:
> 1) Is it used in our kernel?
Nope, not used, as it's said in the commit message.

> if not -- I afraid it will not be compiled
Will be.

> if so -- I doubt it should be exported.
Will be.

> By another words:
> are you sure "EXPORT_SYMBOL(generic_segment_checks);" line is required?
Yes, i'm sure.

i've tested zfs compilation on the test kernel without and with the patch.

> 2) your previous patch-set "fs/iov_iter: Fix ZFS kernel module compilation" is still required too?

Nope, it's not needed anymore.

We can probably drop PAGE_ITER - it's just a cleanup.
i'll send that patch once again.

> Thank you,
> 	Vasily Averin
>
> On 12/29/20 4:40 PM, Konstantin Khorenko wrote:
>> This is a partial revert of a9ca9d715754 ("kill generic_segment_checks()").
>>
>> The patch resurrects the function, but does not restore any call for it.
>>
>> Why this functiona is needed then?
>> For zfs.
>>
>> VZ kernel does not have file_operations::read_iter()/write_iter()
>> compatible with mainstream version thus zfs external module
>> tries live without iters and assumes generic_segment_checks() exists.
>>
>> So keep the generic_segment_checks() specially for zfs.
>>
>> https://bugs.openvz.org/browse/OVZ-7243
>>
>> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
>> ---
>>  include/linux/fs.h |  2 ++
>>  mm/filemap.c       | 39 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 insertions(+)
>>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 8398ec202a42..aee8adfc1252 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -3255,6 +3255,8 @@ extern ssize_t generic_file_buffered_write_iter(struct kiocb *, struct iov_iter
>>  		loff_t, loff_t *, ssize_t);
>>  extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
>>  extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
>> +extern int generic_segment_checks(const struct iovec *iov,
>> +		unsigned long *nr_segs, size_t *count, int access_flags);
>>
>>  /* fs/block_dev.c */
>>  extern ssize_t blkdev_aio_read(struct kiocb *, const struct iovec *,
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index 950d92b6059b..585c57ef1e0a 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -1981,6 +1981,45 @@ int file_read_actor(read_descriptor_t *desc, struct page *page,
>>  	return size;
>>  }
>>
>> +/*
>> + * Performs necessary checks before doing a write
>> + * @iov:	io vector request
>> + * @nr_segs:	number of segments in the iovec
>> + * @count:	number of bytes to write
>> + * @access_flags: type of access: %VERIFY_READ or %VERIFY_WRITE
>> + *
>> + * Adjust number of segments and amount of bytes to write (nr_segs should be
>> + * properly initialized first). Returns appropriate error code that caller
>> + * should return or zero in case that write should be allowed.
>> + */
>> +int generic_segment_checks(const struct iovec *iov,
>> +			unsigned long *nr_segs, size_t *count, int access_flags)
>> +{
>> +	unsigned long   seg;
>> +	size_t cnt = 0;
>> +	for (seg = 0; seg < *nr_segs; seg++) {
>> +		const struct iovec *iv = &iov[seg];
>> +
>> +		/*
>> +		 * If any segment has a negative length, or the cumulative
>> +		 * length ever wraps negative then return -EINVAL.
>> +		 */
>> +		cnt += iv->iov_len;
>> +		if (unlikely((ssize_t)(cnt|iv->iov_len) < 0))
>> +			return -EINVAL;
>> +		if (access_ok(access_flags, iv->iov_base, iv->iov_len))
>> +			continue;
>> +		if (seg == 0)
>> +			return -EFAULT;
>> +		*nr_segs = seg;
>> +		cnt -= iv->iov_len;	/* This segment is no good */
>> +		break;
>> +	}
>> +	*count = cnt;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(generic_segment_checks);
>> +
>>  static int file_read_iter_actor(read_descriptor_t *desc, struct page *page,
>>  				unsigned long offset, unsigned long size)
>>  {
>>
> .
>