| Message ID | 20201229134013.5144-1-khorenko@virtuozzo.com |
|---|---|
| State | New |
| Series | "fs: Resurrect generic_segment_checks()" |
| Headers | show |
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) {
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) > { >
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) >> { >> > . >
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(+)