2024-03-20 11:33:00

by Maximilian Heyne

[permalink] [raw]
Subject: [PATCH] btrfs: allocate btrfs_ioctl_defrag_range_args on stack

From: Goldwyn Rodrigues <[email protected]>

commit c853a5783ebe123847886d432354931874367292 upstream.

Instead of using kmalloc() to allocate btrfs_ioctl_defrag_range_args,
allocate btrfs_ioctl_defrag_range_args on stack, the size is reasonably
small and ioctls are called in process context.

sizeof(btrfs_ioctl_defrag_range_args) = 48

Reviewed-by: Anand Jain <[email protected]>
Signed-off-by: Goldwyn Rodrigues <[email protected]>
Reviewed-by: David Sterba <[email protected]>
Signed-off-by: David Sterba <[email protected]>
CC: [email protected] # 4.14+
[ This patch is needed to fix a memory leak of "range" that was
introduced when commit 173431b274a9 ("btrfs: defrag: reject unknown
flags of btrfs_ioctl_defrag_range_args") was backported to kernels
lacking this patch. Now with these two patches applied in reverse order,
range->flags needed to change back to range.flags.
This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.]
Signed-off-by: Maximilian Heyne <[email protected]>
---
fs/btrfs/ioctl.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 049b837934e5..ab8ed187746e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3148,7 +3148,7 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp)
{
struct inode *inode = file_inode(file);
struct btrfs_root *root = BTRFS_I(inode)->root;
- struct btrfs_ioctl_defrag_range_args *range;
+ struct btrfs_ioctl_defrag_range_args range = {0};
int ret;

ret = mnt_want_write_file(file);
@@ -3180,37 +3180,28 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp)
goto out;
}

- range = kzalloc(sizeof(*range), GFP_KERNEL);
- if (!range) {
- ret = -ENOMEM;
- goto out;
- }
-
if (argp) {
- if (copy_from_user(range, argp,
- sizeof(*range))) {
+ if (copy_from_user(&range, argp, sizeof(range))) {
ret = -EFAULT;
- kfree(range);
goto out;
}
- if (range->flags & ~BTRFS_DEFRAG_RANGE_FLAGS_SUPP) {
+ if (range.flags & ~BTRFS_DEFRAG_RANGE_FLAGS_SUPP) {
ret = -EOPNOTSUPP;
goto out;
}
/* compression requires us to start the IO */
- if ((range->flags & BTRFS_DEFRAG_RANGE_COMPRESS)) {
- range->flags |= BTRFS_DEFRAG_RANGE_START_IO;
- range->extent_thresh = (u32)-1;
+ if ((range.flags & BTRFS_DEFRAG_RANGE_COMPRESS)) {
+ range.flags |= BTRFS_DEFRAG_RANGE_START_IO;
+ range.extent_thresh = (u32)-1;
}
} else {
/* the rest are all set to zero by kzalloc */
- range->len = (u64)-1;
+ range.len = (u64)-1;
}
ret = btrfs_defrag_file(file_inode(file), file,
- range, BTRFS_OLDEST_GENERATION, 0);
+ &range, BTRFS_OLDEST_GENERATION, 0);
if (ret > 0)
ret = 0;
- kfree(range);
break;
default:
ret = -EINVAL;
--
2.40.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879





2024-03-20 16:22:41

by David Sterba

[permalink] [raw]
Subject: Re: [PATCH] btrfs: allocate btrfs_ioctl_defrag_range_args on stack

On Wed, Mar 20, 2024 at 11:31:56AM +0000, Maximilian Heyne wrote:
> From: Goldwyn Rodrigues <[email protected]>
>
> commit c853a5783ebe123847886d432354931874367292 upstream.
>
> Instead of using kmalloc() to allocate btrfs_ioctl_defrag_range_args,
> allocate btrfs_ioctl_defrag_range_args on stack, the size is reasonably
> small and ioctls are called in process context.
>
> sizeof(btrfs_ioctl_defrag_range_args) = 48
>
> Reviewed-by: Anand Jain <[email protected]>
> Signed-off-by: Goldwyn Rodrigues <[email protected]>
> Reviewed-by: David Sterba <[email protected]>
> Signed-off-by: David Sterba <[email protected]>
> CC: [email protected] # 4.14+
> [ This patch is needed to fix a memory leak of "range" that was
> introduced when commit 173431b274a9 ("btrfs: defrag: reject unknown
> flags of btrfs_ioctl_defrag_range_args") was backported to kernels
> lacking this patch. Now with these two patches applied in reverse order,
> range->flags needed to change back to range.flags.
> This bug was discovered and resolved using Coverity Static Analysis
> Security Testing (SAST) by Synopsys, Inc.]
> Signed-off-by: Maximilian Heyne <[email protected]>

Acked-by: David Sterba <[email protected]>

for backport to stable as a prerequisite for 173431b274a9a5 ("btrfs:
defrag: reject unknown flags of btrfs_ioctl_defrag_range_args").

2024-03-29 12:16:25

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] btrfs: allocate btrfs_ioctl_defrag_range_args on stack

On Wed, Mar 20, 2024 at 05:14:33PM +0100, David Sterba wrote:
> On Wed, Mar 20, 2024 at 11:31:56AM +0000, Maximilian Heyne wrote:
> > From: Goldwyn Rodrigues <[email protected]>
> >
> > commit c853a5783ebe123847886d432354931874367292 upstream.
> >
> > Instead of using kmalloc() to allocate btrfs_ioctl_defrag_range_args,
> > allocate btrfs_ioctl_defrag_range_args on stack, the size is reasonably
> > small and ioctls are called in process context.
> >
> > sizeof(btrfs_ioctl_defrag_range_args) = 48
> >
> > Reviewed-by: Anand Jain <[email protected]>
> > Signed-off-by: Goldwyn Rodrigues <[email protected]>
> > Reviewed-by: David Sterba <[email protected]>
> > Signed-off-by: David Sterba <[email protected]>
> > CC: [email protected] # 4.14+
> > [ This patch is needed to fix a memory leak of "range" that was
> > introduced when commit 173431b274a9 ("btrfs: defrag: reject unknown
> > flags of btrfs_ioctl_defrag_range_args") was backported to kernels
> > lacking this patch. Now with these two patches applied in reverse order,
> > range->flags needed to change back to range.flags.
> > This bug was discovered and resolved using Coverity Static Analysis
> > Security Testing (SAST) by Synopsys, Inc.]
> > Signed-off-by: Maximilian Heyne <[email protected]>
>
> Acked-by: David Sterba <[email protected]>
>
> for backport to stable as a prerequisite for 173431b274a9a5 ("btrfs:
> defrag: reject unknown flags of btrfs_ioctl_defrag_range_args").
>

Now queued up, thanks!

greg k-h