| Message ID | 20170712173650.GC4212@magnolia |
|---|---|
| State | Not Applicable |
| Headers | show |
On Jul 12, 2017, at 10:36 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote: > > In the iomap implementations of SEEK_HOLE and SEEK_DATA, make sure we > return -ENXIO for negative offsets instead of badgering the iomap > provider with garbage requests. Hmm, wouldn't it be useful to be able to seek N bytes before the hole or data? It would make more sense to verify the result after finding the hole or data and adding the relative offset. It should only be an error if the resulting offset is before the start or after the actual file end. Of course, if the passed offset > size from the start then there is no way it can get better and that would be grounds for returning early. Cheers, Andreas > Inspired-by: Mateusz S <muttdini@gmail.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/iomap.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/iomap.c b/fs/iomap.c > index 432eed8..16f5c074 100644 > --- a/fs/iomap.c > +++ b/fs/iomap.c > @@ -610,8 +610,8 @@ iomap_seek_hole(struct inode *inode, loff_t offset, const struct iomap_ops *ops) > loff_t length = size - offset; > loff_t ret; > > - /* Nothing to be found beyond the end of the file. */ > - if (offset >= size) > + /* Nothing to be found before or beyond the end of the file. */ > + if (offset < 0 || offset >= size) > return -ENXIO; > > while (length > 0) { > @@ -656,8 +656,8 @@ iomap_seek_data(struct inode *inode, loff_t offset, const struct iomap_ops *ops) > loff_t length = size - offset; > loff_t ret; > > - /* Nothing to be found beyond the end of the file. */ > - if (offset >= size) > + /* Nothing to be found before or beyond the end of the file. */ > + if (offset < 0 || offset >= size) > return -ENXIO; > > while (length > 0) { Cheers, Andreas
On Wed, Jul 12, 2017 at 11:08:51PM -0700, Andreas Dilger wrote: > On Jul 12, 2017, at 10:36 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote: > > > > In the iomap implementations of SEEK_HOLE and SEEK_DATA, make sure we > > return -ENXIO for negative offsets instead of badgering the iomap > > provider with garbage requests. > > Hmm, wouldn't it be useful to be able to seek N bytes before the hole > or data? Yes, but you can do that with a first lseek SEEK_DATA call to position us at the start of data followed by a second lseek SEEK_CUR to position us at the desired before-data offset. The offset argument to SEEK DATA tells us where to start looking for the next hole/not-hole. > It would make more sense to verify the result after finding the hole > or data and adding the relative offset. It should only be an error if > the resulting offset is before the start or after the actual file end. In any case, the manpage says: "SEEK_DATA Adjust the file offset to the next location in the file greater than or equal to offset containing data. If offset points to data, then the file offset is set to offset. "SEEK_HOLE Adjust the file offset to the next hole in the file greater than or equal to offset. If offset points into the middle of a hole, then the file offset is set to offset. If there is no hole past offset, then the file offset is adjusted to the end of the file (i.e., there is an implicit hole at the end of any file)." ...which to my mind means that offset has to be an absolute file offset. Therefore, negative offsets aren't allowed, and we're stuck with that. Regrettably the ERRORS section doesn't specify the behavior when offset is negative, so this patch merely fixes XFS to behave the same way it did up to 4.12 (which fwiw matches btrfs behavior). --D > Of course, if the passed offset > size from the start then > there is no way it can get better and that would be grounds for > returning early. > > Cheers, Andreas > > > Inspired-by: Mateusz S <muttdini@gmail.com> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > fs/iomap.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/fs/iomap.c b/fs/iomap.c > > index 432eed8..16f5c074 100644 > > --- a/fs/iomap.c > > +++ b/fs/iomap.c > > @@ -610,8 +610,8 @@ iomap_seek_hole(struct inode *inode, loff_t offset, const struct iomap_ops *ops) > > loff_t length = size - offset; > > loff_t ret; > > > > - /* Nothing to be found beyond the end of the file. */ > > - if (offset >= size) > > + /* Nothing to be found before or beyond the end of the file. */ > > + if (offset < 0 || offset >= size) > > return -ENXIO; > > > > while (length > 0) { > > @@ -656,8 +656,8 @@ iomap_seek_data(struct inode *inode, loff_t offset, const struct iomap_ops *ops) > > loff_t length = size - offset; > > loff_t ret; > > > > - /* Nothing to be found beyond the end of the file. */ > > - if (offset >= size) > > + /* Nothing to be found before or beyond the end of the file. */ > > + if (offset < 0 || offset >= size) > > return -ENXIO; > > > > while (length > 0) { > > > Cheers, Andreas > > > > >
diff --git a/fs/iomap.c b/fs/iomap.c index 432eed8..16f5c074 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -610,8 +610,8 @@ iomap_seek_hole(struct inode *inode, loff_t offset, const struct iomap_ops *ops) loff_t length = size - offset; loff_t ret; - /* Nothing to be found beyond the end of the file. */ - if (offset >= size) + /* Nothing to be found before or beyond the end of the file. */ + if (offset < 0 || offset >= size) return -ENXIO; while (length > 0) { @@ -656,8 +656,8 @@ iomap_seek_data(struct inode *inode, loff_t offset, const struct iomap_ops *ops) loff_t length = size - offset; loff_t ret; - /* Nothing to be found beyond the end of the file. */ - if (offset >= size) + /* Nothing to be found before or beyond the end of the file. */ + if (offset < 0 || offset >= size) return -ENXIO; while (length > 0) {
In the iomap implementations of SEEK_HOLE and SEEK_DATA, make sure we return -ENXIO for negative offsets instead of badgering the iomap provider with garbage requests. Inspired-by: Mateusz S <muttdini@gmail.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/iomap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)