linux-kernel - Re: [PATCH v1 3/3] btrfs: replace kmalloc() and memcpy() with kmemdup()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <35055563-5280-4f57-ad9c-27f47d94f54a@gmail.com>
Date: Wed, 18 Dec 2024 18:03:48 +0100
From: Mirsad Todorovac <mtodorovac69@...il.com>
To: Mark Harmstone <maharmstone@...a.com>,
 Alex Deucher <alexander.deucher@....com>,
 Victor Skvortsov <victor.skvortsov@....com>,
 "amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: Christian König <christian.koenig@....com>,
 Xinhui Pan <Xinhui.Pan@....com>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Chris Mason <clm@...a.com>,
 Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>,
 "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>
Subject: Re: [PATCH v1 3/3] btrfs: replace kmalloc() and memcpy() with
 kmemdup()

Hi,

Yes, I see you have the prior work and I have duplicated your work.

Apology for the inconvenience.

Best regards,
Mirsad Todorovac

On 12/18/24 11:32, Mark Harmstone wrote:
> There's a fix for this already in the for-next branch:
> https://github.com/btrfs/linux/commit/1a287050962c6847fa4918d6b2a0f4cee35c6943
> 
> On 17/12/24 22:58, Mirsad Todorovac wrote:
>>>
>> The static analyser tool gave the following advice:
>>
>> ./fs/btrfs/ioctl.c:4987:9-16: WARNING opportunity for kmemdup
>>
>>     4986                 if (!iov) {
>>   → 4987                         iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS);
>>     4988                         if (!iov) {
>>     4989                                 unlock_extent(io_tree, start, lockend, &cached_state);
>>     4990                                 btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
>>     4991                                 ret = -ENOMEM;
>>     4992                                 goto out_acct;
>>     4993                         }
>>     4994
>>   → 4995                         memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt);
>>     4996                 }
>>
>> Replacing kmalloc() + memcpy() with kmemdump() doesn't change semantics.
>> Original code works without fault, so this is not a bug fix but proposed improvement.
>>
>> Link: https://urldefense.com/v3/__https://lwn.net/Articles/198928/__;!!Bt8RZUm9aw!4OVzQmIUbyH-UGdUwMAL582hR4Q-7HN2fn9IpyxeA1T8qrcC8RdBVz4xuL4m35_kksUllAi6OmdbRehcFpwfHw$
>> Fixes: 34310c442e175 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)")
>> Cc: Chris Mason <clm@...com>
>> Cc: Josef Bacik <josef@...icpanda.com>
>> Cc: David Sterba <dsterba@...e.com>
>> Cc: linux-btrfs@...r.kernel.org
>> Cc: linux-kernel@...r.kernel.org
>> Signed-off-by: Mirsad Todorovac <mtodorovac69@...il.com>
>> ---
>>   v1:
>> 	initial version.
>>
>>   fs/btrfs/ioctl.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 3af8bb0c8d75..c2f769334d3c 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -4984,15 +4984,13 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
>>   		 * undo this.
>>   		 */
>>   		if (!iov) {
>> -			iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS);
>> +			iov = kmemdup(iovstack, sizeof(struct iovec) * args.iovcnt, GFP_NOFS);
>>   			if (!iov) {
>>   				unlock_extent(io_tree, start, lockend, &cached_state);
>>   				btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
>>   				ret = -ENOMEM;
>>   				goto out_acct;
>>   			}
>> -
>> -			memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt);
>>   		}
>>   
>>   		count = min_t(u64, iov_iter_count(&iter), disk_io_size);
>