diff mbox series

[v2] fs: btrfs: fix out of bounds write

Message ID 20240618214138.3212175-1-alexthreed@gmail.com
State Accepted
Commit ee1941e4fec601a8444f49c7dad04ad700d501a6
Delegated to: Tom Rini
Headers show
Series [v2] fs: btrfs: fix out of bounds write | expand

Commit Message

Alex ThreeD June 18, 2024, 9:41 p.m. UTC
Fix btrfs_read/read_and_truncate_page write out of bounds of destination
buffer. Old behavior break bootstd malloc'd buffers of exact file size.
Previously this OOB write have not been noticed because distroboot usually
read files into huge static memory areas.

Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
Fixes: e342718 ("fs: btrfs: Implement btrfs_file_read()")
---

Changes in v2:
- fix error path handling
- add Fixes tag
- use min3

 fs/btrfs/inode.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Qu Wenruo June 18, 2024, 10:21 p.m. UTC | #1
在 2024/6/19 07:11, Alex Shumsky 写道:
> Fix btrfs_read/read_and_truncate_page write out of bounds of destination
> buffer. Old behavior break bootstd malloc'd buffers of exact file size.
> Previously this OOB write have not been noticed because distroboot usually
> read files into huge static memory areas.
>
> Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
> Fixes: e342718 ("fs: btrfs: Implement btrfs_file_read()")

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>
> Changes in v2:
> - fix error path handling
> - add Fixes tag
> - use min3
>
>   fs/btrfs/inode.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4691612eda..3998ffc2c8 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -640,7 +640,11 @@ static int read_and_truncate_page(struct btrfs_path *path,
>   	extent_type = btrfs_file_extent_type(leaf, fi);
>   	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>   		ret = btrfs_read_extent_inline(path, fi, buf);
> -		memcpy(dest, buf + page_off, min(page_len, ret));
> +		if (ret < 0) {
> +			free(buf);
> +			return ret;
> +		}
> +		memcpy(dest, buf + page_off, min3(page_len, ret, len));
>   		free(buf);
>   		return len;
>   	}
> @@ -652,7 +656,7 @@ static int read_and_truncate_page(struct btrfs_path *path,
>   		free(buf);
>   		return ret;
>   	}
> -	memcpy(dest, buf + page_off, page_len);
> +	memcpy(dest, buf + page_off, min(page_len, len));
>   	free(buf);
>   	return len;
>   }
Alex ThreeD June 24, 2024, 10:22 p.m. UTC | #2
Hi all,

Is there something on my side needed to push this forward?

On Wed, Jun 19, 2024 at 12:41 AM Alex Shumsky <alexthreed@gmail.com> wrote:

> Fix btrfs_read/read_and_truncate_page write out of bounds of destination
> buffer. Old behavior break bootstd malloc'd buffers of exact file size.
> Previously this OOB write have not been noticed because distroboot usually
> read files into huge static memory areas.
>
> Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
> Fixes: e342718 ("fs: btrfs: Implement btrfs_file_read()")
> ---
>
> Changes in v2:
> - fix error path handling
> - add Fixes tag
> - use min3
>
>  fs/btrfs/inode.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4691612eda..3998ffc2c8 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -640,7 +640,11 @@ static int read_and_truncate_page(struct btrfs_path
> *path,
>         extent_type = btrfs_file_extent_type(leaf, fi);
>         if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>                 ret = btrfs_read_extent_inline(path, fi, buf);
> -               memcpy(dest, buf + page_off, min(page_len, ret));
> +               if (ret < 0) {
> +                       free(buf);
> +                       return ret;
> +               }
> +               memcpy(dest, buf + page_off, min3(page_len, ret, len));
>                 free(buf);
>                 return len;
>         }
> @@ -652,7 +656,7 @@ static int read_and_truncate_page(struct btrfs_path
> *path,
>                 free(buf);
>                 return ret;
>         }
> -       memcpy(dest, buf + page_off, page_len);
> +       memcpy(dest, buf + page_off, min(page_len, len));
>         free(buf);
>         return len;
>  }
> --
> 2.34.1
>
>
Tom Rini June 24, 2024, 10:25 p.m. UTC | #3
On Tue, Jun 25, 2024 at 01:22:01AM +0300, Alex ThreeD wrote:

> Hi all,
> 
> Is there something on my side needed to push this forward?

I will pick it up for -next in a while, thanks.

> 
> On Wed, Jun 19, 2024 at 12:41 AM Alex Shumsky <alexthreed@gmail.com> wrote:
> 
> > Fix btrfs_read/read_and_truncate_page write out of bounds of destination
> > buffer. Old behavior break bootstd malloc'd buffers of exact file size.
> > Previously this OOB write have not been noticed because distroboot usually
> > read files into huge static memory areas.
> >
> > Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
> > Fixes: e342718 ("fs: btrfs: Implement btrfs_file_read()")
> > ---
> >
> > Changes in v2:
> > - fix error path handling
> > - add Fixes tag
> > - use min3
> >
> >  fs/btrfs/inode.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 4691612eda..3998ffc2c8 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -640,7 +640,11 @@ static int read_and_truncate_page(struct btrfs_path
> > *path,
> >         extent_type = btrfs_file_extent_type(leaf, fi);
> >         if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> >                 ret = btrfs_read_extent_inline(path, fi, buf);
> > -               memcpy(dest, buf + page_off, min(page_len, ret));
> > +               if (ret < 0) {
> > +                       free(buf);
> > +                       return ret;
> > +               }
> > +               memcpy(dest, buf + page_off, min3(page_len, ret, len));
> >                 free(buf);
> >                 return len;
> >         }
> > @@ -652,7 +656,7 @@ static int read_and_truncate_page(struct btrfs_path
> > *path,
> >                 free(buf);
> >                 return ret;
> >         }
> > -       memcpy(dest, buf + page_off, page_len);
> > +       memcpy(dest, buf + page_off, min(page_len, len));
> >         free(buf);
> >         return len;
> >  }
> > --
> > 2.34.1
> >
> >
Tom Rini June 28, 2024, 7:49 p.m. UTC | #4
On Wed, 19 Jun 2024 00:41:38 +0300, Alex Shumsky wrote:

> Fix btrfs_read/read_and_truncate_page write out of bounds of destination
> buffer. Old behavior break bootstd malloc'd buffers of exact file size.
> Previously this OOB write have not been noticed because distroboot usually
> read files into huge static memory areas.
> 
> 

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 4691612eda..3998ffc2c8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -640,7 +640,11 @@  static int read_and_truncate_page(struct btrfs_path *path,
 	extent_type = btrfs_file_extent_type(leaf, fi);
 	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
 		ret = btrfs_read_extent_inline(path, fi, buf);
-		memcpy(dest, buf + page_off, min(page_len, ret));
+		if (ret < 0) {
+			free(buf);
+			return ret;
+		}
+		memcpy(dest, buf + page_off, min3(page_len, ret, len));
 		free(buf);
 		return len;
 	}
@@ -652,7 +656,7 @@  static int read_and_truncate_page(struct btrfs_path *path,
 		free(buf);
 		return ret;
 	}
-	memcpy(dest, buf + page_off, page_len);
+	memcpy(dest, buf + page_off, min(page_len, len));
 	free(buf);
 	return len;
 }