* [PATCH] btrfs: fix iomap_begin length for nocow writes
@ 2023-06-08 9:10 Christoph Hellwig
2023-06-09 16:56 ` Filipe Manana
2023-06-09 22:17 ` David Sterba
0 siblings, 2 replies; 3+ messages in thread
From: Christoph Hellwig @ 2023-06-08 9:10 UTC (permalink / raw)
To: clm, josef, dsterba; +Cc: nborisov, linux-btrfs, syzbot+ee90502d5c8fd1d0dd93
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(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: fix iomap_begin length for nocow writes
2023-06-08 9:10 [PATCH] btrfs: fix iomap_begin length for nocow writes Christoph Hellwig
@ 2023-06-09 16:56 ` Filipe Manana
2023-06-09 22:17 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: Filipe Manana @ 2023-06-09 16:56 UTC (permalink / raw)
To: Christoph Hellwig
Cc: clm, josef, dsterba, nborisov, linux-btrfs,
syzbot+ee90502d5c8fd1d0dd93
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: fix iomap_begin length for nocow writes
2023-06-08 9:10 [PATCH] btrfs: fix iomap_begin length for nocow writes Christoph Hellwig
2023-06-09 16:56 ` Filipe Manana
@ 2023-06-09 22:17 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2023-06-09 22:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: clm, josef, dsterba, nborisov, linux-btrfs,
syzbot+ee90502d5c8fd1d0dd93
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-09 22:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-08 9:10 [PATCH] btrfs: fix iomap_begin length for nocow writes Christoph Hellwig
2023-06-09 16:56 ` Filipe Manana
2023-06-09 22:17 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox