| Message ID | 20230608091025.104716-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix iomap_begin length for nocow writes | expand |
On Thu, Jun 8, 2023 at 10:46 AM Christoph Hellwig <hch@lst.de> wrote: > > can_nocow_extent can reduce the len passed in, which needs to be > propagated to btrfs_dio_iomap_begin so that iomap does not submit > more data then is mapped. > > This problems exists since the btrfs_get_blocks_direct helper was added > in commit c5794e51784a ("btrfs: Factor out write portion of > btrfs_get_blocks_direct"), but the ordered_extent splitting added in > commit b73a6fd1b1ef ("btrfs: split partial dio bios before submit") > added a WARN_ON that made a syzcaller test fail. > > Fixes: c5794e51784a ("btrfs: Factor out write portion of btrfs_get_blocks_direct") > Reported-by: syzbot+ee90502d5c8fd1d0dd93@syzkaller.appspotmail.com > Signed-off-by: Christoph Hellwig <hch@lst.de> > Tested-by: syzbot+ee90502d5c8fd1d0dd93@syzkaller.appspotmail.com Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks good to me, thanks. > --- > fs/btrfs/inode.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 19c707bc8801a9..3f99f02dc1fe20 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -7264,7 +7264,7 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, > static int btrfs_get_blocks_direct_write(struct extent_map **map, > struct inode *inode, > struct btrfs_dio_data *dio_data, > - u64 start, u64 len, > + u64 start, u64 *lenp, > unsigned int iomap_flags) > { > const bool nowait = (iomap_flags & IOMAP_NOWAIT); > @@ -7275,6 +7275,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map, > struct btrfs_block_group *bg; > bool can_nocow = false; > bool space_reserved = false; > + u64 len = *lenp; > u64 prev_len; > int ret = 0; > > @@ -7345,15 +7346,19 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map, > free_extent_map(em); > *map = NULL; > > - if (nowait) > - return -EAGAIN; > + if (nowait) { > + ret = -EAGAIN; > + goto out; > + } > > /* > * If we could not allocate data space before locking the file > * range and we can't do a NOCOW write, then we have to fail. > */ > - if (!dio_data->data_space_reserved) > - return -ENOSPC; > + if (!dio_data->data_space_reserved) { > + ret = -ENOSPC; > + goto out; > + } > > /* > * We have to COW and we have already reserved data space before, > @@ -7394,6 +7399,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map, > btrfs_delalloc_release_extents(BTRFS_I(inode), len); > btrfs_delalloc_release_metadata(BTRFS_I(inode), len, true); > } > + *lenp = len; > return ret; > } > > @@ -7570,7 +7576,7 @@ static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start, > > if (write) { > ret = btrfs_get_blocks_direct_write(&em, inode, dio_data, > - start, len, flags); > + start, &len, flags); > if (ret < 0) > goto unlock_err; > unlock_extents = true; > -- > 2.39.2 >
On Thu, Jun 08, 2023 at 11:10:25AM +0200, Christoph Hellwig wrote: > can_nocow_extent can reduce the len passed in, which needs to be > propagated to btrfs_dio_iomap_begin so that iomap does not submit > more data then is mapped. > > This problems exists since the btrfs_get_blocks_direct helper was added > in commit c5794e51784a ("btrfs: Factor out write portion of > btrfs_get_blocks_direct"), but the ordered_extent splitting added in > commit b73a6fd1b1ef ("btrfs: split partial dio bios before submit") > added a WARN_ON that made a syzcaller test fail. > > Fixes: c5794e51784a ("btrfs: Factor out write portion of btrfs_get_blocks_direct") > Reported-by: syzbot+ee90502d5c8fd1d0dd93@syzkaller.appspotmail.com > Signed-off-by: Christoph Hellwig <hch@lst.de> > Tested-by: syzbot+ee90502d5c8fd1d0dd93@syzkaller.appspotmail.com Added to misc-next, thanks.
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 19c707bc8801a9..3f99f02dc1fe20 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -7264,7 +7264,7 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, static int btrfs_get_blocks_direct_write(struct extent_map **map, struct inode *inode, struct btrfs_dio_data *dio_data, - u64 start, u64 len, + u64 start, u64 *lenp, unsigned int iomap_flags) { const bool nowait = (iomap_flags & IOMAP_NOWAIT); @@ -7275,6 +7275,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map, struct btrfs_block_group *bg; bool can_nocow = false; bool space_reserved = false; + u64 len = *lenp; u64 prev_len; int ret = 0; @@ -7345,15 +7346,19 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map, free_extent_map(em); *map = NULL; - if (nowait) - return -EAGAIN; + if (nowait) { + ret = -EAGAIN; + goto out; + } /* * If we could not allocate data space before locking the file * range and we can't do a NOCOW write, then we have to fail. */ - if (!dio_data->data_space_reserved) - return -ENOSPC; + if (!dio_data->data_space_reserved) { + ret = -ENOSPC; + goto out; + } /* * We have to COW and we have already reserved data space before, @@ -7394,6 +7399,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map, btrfs_delalloc_release_extents(BTRFS_I(inode), len); btrfs_delalloc_release_metadata(BTRFS_I(inode), len, true); } + *lenp = len; return ret; } @@ -7570,7 +7576,7 @@ static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start, if (write) { ret = btrfs_get_blocks_direct_write(&em, inode, dio_data, - start, len, flags); + start, &len, flags); if (ret < 0) goto unlock_err; unlock_extents = true;
can_nocow_extent can reduce the len passed in, which needs to be propagated to btrfs_dio_iomap_begin so that iomap does not submit more data then is mapped. This problems exists since the btrfs_get_blocks_direct helper was added in commit c5794e51784a ("btrfs: Factor out write portion of btrfs_get_blocks_direct"), but the ordered_extent splitting added in commit b73a6fd1b1ef ("btrfs: split partial dio bios before submit") added a WARN_ON that made a syzcaller test fail. Fixes: c5794e51784a ("btrfs: Factor out write portion of btrfs_get_blocks_direct") Reported-by: syzbot+ee90502d5c8fd1d0dd93@syzkaller.appspotmail.com Signed-off-by: Christoph Hellwig <hch@lst.de> Tested-by: syzbot+ee90502d5c8fd1d0dd93@syzkaller.appspotmail.com --- fs/btrfs/inode.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)