On Tue, Sep 05, 2023 at 02:41:20PM +0200, Christoph Hellwig wrote:
> iomap_to_bh currently BUG()s when the passed in block number is not
> in the iomap. For file systems that have proper synchronization this
> should never happen and so far hasn't in mainline, but for block devices
> size changes aren't fully synchronized against ongoing I/O. Instead
> of BUG()ing in this case, return -EIO to the caller, which already has
> proper error handling. While we're at it, also return -EIO for an
> unknown iomap state instead of returning garbage.
>
> Fixes: 487c607df790 ("block: use iomap for writes to block devices")
> Reported-by: syzbot+4a08ffdf3667b36650a1@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
Looks like a good improvement. Who should this go through, me (iomap)
or viro/brauner (vfs*) ?
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
(lol is email down again?)
--D
> fs/buffer.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 2379564e5aeadf..a6785cd07081cb 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2011,7 +2011,7 @@ void folio_zero_new_buffers(struct folio *folio, size_t from, size_t to)
> }
> EXPORT_SYMBOL(folio_zero_new_buffers);
>
> -static void
> +static int
> iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh,
> const struct iomap *iomap)
> {
> @@ -2025,7 +2025,8 @@ iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh,
> * current block, then do not map the buffer and let the caller
> * handle it.
> */
> - BUG_ON(offset >= iomap->offset + iomap->length);
> + if (offset >= iomap->offset + iomap->length)
> + return -EIO;
>
> switch (iomap->type) {
> case IOMAP_HOLE:
> @@ -2037,7 +2038,7 @@ iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh,
> if (!buffer_uptodate(bh) ||
> (offset >= i_size_read(inode)))
> set_buffer_new(bh);
> - break;
> + return 0;
> case IOMAP_DELALLOC:
> if (!buffer_uptodate(bh) ||
> (offset >= i_size_read(inode)))
> @@ -2045,7 +2046,7 @@ iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh,
> set_buffer_uptodate(bh);
> set_buffer_mapped(bh);
> set_buffer_delay(bh);
> - break;
> + return 0;
> case IOMAP_UNWRITTEN:
> /*
> * For unwritten regions, we always need to ensure that regions
> @@ -2062,7 +2063,10 @@ iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh,
> bh->b_blocknr = (iomap->addr + offset - iomap->offset) >>
> inode->i_blkbits;
> set_buffer_mapped(bh);
> - break;
> + return 0;
> + default:
> + WARN_ON_ONCE(1);
> + return -EIO;
> }
> }
>
> @@ -2103,13 +2107,12 @@ int __block_write_begin_int(struct folio *folio, loff_t pos, unsigned len,
> clear_buffer_new(bh);
> if (!buffer_mapped(bh)) {
> WARN_ON(bh->b_size != blocksize);
> - if (get_block) {
> + if (get_block)
> err = get_block(inode, block, bh, 1);
> - if (err)
> - break;
> - } else {
> - iomap_to_bh(inode, block, bh, iomap);
> - }
> + else
> + err = iomap_to_bh(inode, block, bh, iomap);
> + if (err)
> + break;
>
> if (buffer_new(bh)) {
> clean_bdev_bh_alias(bh);
> --
> 2.39.2
>
