* [PATCH] fs: fix iomap_bmap position calculation
@ 2018-08-02 20:03 Eric Sandeen
2018-08-02 20:08 ` Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Eric Sandeen @ 2018-08-02 20:03 UTC (permalink / raw)
To: linux-xfs, fsdevel; +Cc: Christoph Hellwig
The position calculation in iomap_bmap() shifts bno the wrong way,
so we don't progress properly and end up re-mapping block zero
over and over, yielding an unchanging physical block range as the
logical block advances:
# filefrag -Be file
ext: logical_offset: physical_offset: length: expected: flags:
0: 0.. 0: 21.. 21: 1: merged
1: 1.. 1: 21.. 21: 1: 22: merged
Discontinuity: Block 1 is at 21 (was 22)
2: 2.. 2: 21.. 21: 1: 22: merged
Discontinuity: Block 2 is at 21 (was 22)
3: 3.. 3: 21.. 21: 1: 22: merged
This breaks the FIBMAP interface for anyone using it (XFS), which
in turn breaks LILO, zipl, etc.
Bug-actually-spotted-by: Darrick J. Wong <darrick.wong@oracle.com>
Fixes: 89eb1906a953 ("iomap: add an iomap-based bmap implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fs/iomap.c b/fs/iomap.c
index 77397b5a96ef..0d0bd8845586 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1443,7 +1443,7 @@ iomap_bmap(struct address_space *mapping, sector_t bno,
const struct iomap_ops *ops)
{
struct inode *inode = mapping->host;
- loff_t pos = bno >> inode->i_blkbits;
+ loff_t pos = bno << inode->i_blkbits;
unsigned blocksize = i_blocksize(inode);
if (filemap_write_and_wait(mapping))
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: fix iomap_bmap position calculation
2018-08-02 20:03 [PATCH] fs: fix iomap_bmap position calculation Eric Sandeen
@ 2018-08-02 20:08 ` Darrick J. Wong
2018-08-02 21:02 ` Darrick J. Wong
2018-08-03 15:24 ` Christoph Hellwig
2 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2018-08-02 20:08 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, fsdevel, Christoph Hellwig
On Thu, Aug 02, 2018 at 03:03:51PM -0500, Eric Sandeen wrote:
> The position calculation in iomap_bmap() shifts bno the wrong way,
> so we don't progress properly and end up re-mapping block zero
> over and over, yielding an unchanging physical block range as the
> logical block advances:
>
> # filefrag -Be file
> ext: logical_offset: physical_offset: length: expected: flags:
> 0: 0.. 0: 21.. 21: 1: merged
> 1: 1.. 1: 21.. 21: 1: 22: merged
> Discontinuity: Block 1 is at 21 (was 22)
> 2: 2.. 2: 21.. 21: 1: 22: merged
> Discontinuity: Block 2 is at 21 (was 22)
> 3: 3.. 3: 21.. 21: 1: 22: merged
>
> This breaks the FIBMAP interface for anyone using it (XFS), which
> in turn breaks LILO, zipl, etc.
>
> Bug-actually-spotted-by: Darrick J. Wong <darrick.wong@oracle.com>
> Fixes: 89eb1906a953 ("iomap: add an iomap-based bmap implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 77397b5a96ef..0d0bd8845586 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1443,7 +1443,7 @@ iomap_bmap(struct address_space *mapping, sector_t bno,
> const struct iomap_ops *ops)
> {
> struct inode *inode = mapping->host;
> - loff_t pos = bno >> inode->i_blkbits;
> + loff_t pos = bno << inode->i_blkbits;
> unsigned blocksize = i_blocksize(inode);
>
> if (filemap_write_and_wait(mapping))
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: fix iomap_bmap position calculation
2018-08-02 20:03 [PATCH] fs: fix iomap_bmap position calculation Eric Sandeen
2018-08-02 20:08 ` Darrick J. Wong
@ 2018-08-02 21:02 ` Darrick J. Wong
2018-08-03 15:24 ` Christoph Hellwig
2 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2018-08-02 21:02 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, fsdevel, Christoph Hellwig
On Thu, Aug 02, 2018 at 03:03:51PM -0500, Eric Sandeen wrote:
> The position calculation in iomap_bmap() shifts bno the wrong way,
> so we don't progress properly and end up re-mapping block zero
> over and over, yielding an unchanging physical block range as the
> logical block advances:
>
> # filefrag -Be file
> ext: logical_offset: physical_offset: length: expected: flags:
> 0: 0.. 0: 21.. 21: 1: merged
> 1: 1.. 1: 21.. 21: 1: 22: merged
> Discontinuity: Block 1 is at 21 (was 22)
> 2: 2.. 2: 21.. 21: 1: 22: merged
> Discontinuity: Block 2 is at 21 (was 22)
> 3: 3.. 3: 21.. 21: 1: 22: merged
>
> This breaks the FIBMAP interface for anyone using it (XFS), which
> in turn breaks LILO, zipl, etc.
>
> Bug-actually-spotted-by: Darrick J. Wong <darrick.wong@oracle.com>
> Fixes: 89eb1906a953 ("iomap: add an iomap-based bmap implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
I coulda sworn I already RVBd this, but perhaps not?
Will test it out, anyway...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 77397b5a96ef..0d0bd8845586 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1443,7 +1443,7 @@ iomap_bmap(struct address_space *mapping, sector_t bno,
> const struct iomap_ops *ops)
> {
> struct inode *inode = mapping->host;
> - loff_t pos = bno >> inode->i_blkbits;
> + loff_t pos = bno << inode->i_blkbits;
> unsigned blocksize = i_blocksize(inode);
>
> if (filemap_write_and_wait(mapping))
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: fix iomap_bmap position calculation
2018-08-02 20:03 [PATCH] fs: fix iomap_bmap position calculation Eric Sandeen
2018-08-02 20:08 ` Darrick J. Wong
2018-08-02 21:02 ` Darrick J. Wong
@ 2018-08-03 15:24 ` Christoph Hellwig
2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2018-08-03 15:24 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, fsdevel, Christoph Hellwig
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
I'm surprised nothing caught this earlier..
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-03 17:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-02 20:03 [PATCH] fs: fix iomap_bmap position calculation Eric Sandeen
2018-08-02 20:08 ` Darrick J. Wong
2018-08-02 21:02 ` Darrick J. Wong
2018-08-03 15:24 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.