On 'btrfs_ioctl_qgroup_assign' we first duplicate the argument as
provided by the user, which is kfree'd in the end. But this was not the
case when allocating memory for 'prealloc'. In this case, if it somehow
failed, then the previous code would go directly into calling
'mnt_drop_write_file', without freeing the string duplicated from the
user space.

Fixes: 4addc1ffd67a ("btrfs: qgroup: preallocate memory before adding a relation")
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
---
 fs/btrfs/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 185bef0df1c2..8cb7d5a462ef 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3740,7 +3740,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg)
 		prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL);
 		if (!prealloc) {
 			ret = -ENOMEM;
-			goto drop_write;
+			goto out;
 		}
 	}

--
2.51.0

On Thu, Sep 25, 2025 at 11:42 PM Miquel Sabaté Solà <mssola@mssola.com> wrote:
>
> On 'btrfs_ioctl_qgroup_assign' we first duplicate the argument as
> provided by the user, which is kfree'd in the end. But this was not the
> case when allocating memory for 'prealloc'. In this case, if it somehow
> failed, then the previous code would go directly into calling
> 'mnt_drop_write_file', without freeing the string duplicated from the
> user space.
>
> Fixes: 4addc1ffd67a ("btrfs: qgroup: preallocate memory before adding a relation")
> Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

I pushed it  into the for-next branch [1] with a changed subject to:

btrfs: fix memory leak on duplicated memory in the qgroup assign ioctl

Note that we don't capitalize the first word after the prefix in the subject.
I also made it more specific by mentioning which ioctl, since we have many.

Thanks.

[1] https://github.com/btrfs/linux/commits/for-next/

> ---
>  fs/btrfs/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 185bef0df1c2..8cb7d5a462ef 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3740,7 +3740,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg)
>                 prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL);
>                 if (!prealloc) {
>                         ret = -ENOMEM;
> -                       goto drop_write;
> +                       goto out;
>                 }
>         }
>
> --
> 2.51.0
>

Filipe Manana @ 2025-09-26 10:04 +01:

> On Thu, Sep 25, 2025 at 11:42 PM Miquel Sabaté Solà <mssola@mssola.com> wrote:
>>
>> On 'btrfs_ioctl_qgroup_assign' we first duplicate the argument as
>> provided by the user, which is kfree'd in the end. But this was not the
>> case when allocating memory for 'prealloc'. In this case, if it somehow
>> failed, then the previous code would go directly into calling
>> 'mnt_drop_write_file', without freeing the string duplicated from the
>> user space.
>>
>> Fixes: 4addc1ffd67a ("btrfs: qgroup: preallocate memory before adding a relation")
>> Reviewed-by: Boris Burkov <boris@bur.io>
>> Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> I pushed it  into the for-next branch [1] with a changed subject to:
>
> btrfs: fix memory leak on duplicated memory in the qgroup assign ioctl
>
> Note that we don't capitalize the first word after the prefix in the subject.
> I also made it more specific by mentioning which ioctl, since we have many.

Understood! Thanks for applying the patch.

>
> Thanks.
>
> [1] https://github.com/btrfs/linux/commits/for-next/
>
>> ---
>>  fs/btrfs/ioctl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 185bef0df1c2..8cb7d5a462ef 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -3740,7 +3740,7 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg)
>>                 prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL);
>>                 if (!prealloc) {
>>                         ret = -ENOMEM;
>> -                       goto drop_write;
>> +                       goto out;
>>                 }
>>         }
>>
>> --
>> 2.51.0
>>

Cheers,
Miquel