netdev - Re: [Intel-wired-lan] [PATCH net-next v3] ice: Adjust over allocation of memory in ice_sched_add_root_node() and ice_sched_add

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: <23d2e91c-4215-4ea5-8b3c-4dd58a1062af@molgen.mpg.de>
Date: Tue, 9 Jul 2024 10:54:25 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Aleksandr Mishin <amishin@...rgos.ru>
Cc: lvc-project@...uxtesting.org,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Eric Dumazet <edumazet@...gle.com>, linux-kernel@...r.kernel.org,
 Jakub Kicinski <kuba@...nel.org>, intel-wired-lan@...ts.osuosl.org,
 Simon Horman <horms@...nel.org>, netdev@...r.kernel.org,
 Tony Nguyen <anthony.l.nguyen@...el.com>, Paolo Abeni <pabeni@...hat.com>,
 "David S. Miller" <davem@...emloft.net>
Subject: Re: [Intel-wired-lan] [PATCH net-next v3] ice: Adjust over allocation
 of memory in ice_sched_add_root_node() and ice_sched_add_node()

[Cc: -anirudh.venkataramanan@...el.com (Address rejected)]

Am 09.07.24 um 10:49 schrieb Paul Menzel:
> Dear Aleksandr,
> 
> 
> Thank you for your patch.
> 
> 
> Am 08.07.24 um 20:27 schrieb Aleksandr Mishin:
>> In ice_sched_add_root_node() and ice_sched_add_node() there are calls to
>> devm_kcalloc() in order to allocate memory for array of pointers to
>> 'ice_sched_node' structure. But incorrect types are used as sizeof()
>> arguments in these calls (structures instead of pointers) which leads to
>> over allocation of memory.
> 
> If you have the explicit size at hand, it’d be great if you added those 
> to the commit message.
> 
>> Adjust over allocation of memory by correcting types in devm_kcalloc()
>> sizeof() arguments.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Maybe mention, that Coverity found that too, and the warning was 
> disabled, and use that commit in Fixes: tag? That’d be commit 
> b36c598c999c (ice: Updates to Tx scheduler code), different from the one 
> you used.
> 
> `Documentation/process/submitting-patches.rst` says:
> 
>> A Fixes: tag indicates that the patch fixes an issue in a previous
>> commit. It is used to make it easy to determine where a bug
>> originated, which can help review a bug fix. This tag also assists
>> the stable kernel team in determining which stable kernel versions
>> should receive your fix. This is the preferred method for indicating
>> a bug fixed by the patch.
> 
> 
>> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> Signed-off-by: Aleksandr Mishin <amishin@...rgos.ru>
>> ---
>> v3:
>>    - Update comment and use the correct entities as suggested by Przemek
>> v2: https://lore.kernel.org/all/20240706140518.9214-1-amishin@t-argos.ru/
>>    - Update comment, remove 'Fixes' tag and change the tree from 'net' to
>>      'net-next' as suggested by Simon
>>      (https://lore.kernel.org/all/20240706095258.GB1481495@kernel.org/)
>> v1: 
>> https://lore.kernel.org/all/20240705163620.12429-1-amishin@t-argos.ru/
>>
>>   drivers/net/ethernet/intel/ice/ice_sched.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c 
>> b/drivers/net/ethernet/intel/ice/ice_sched.c
>> index ecf8f5d60292..6ca13c5dcb14 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_sched.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_sched.c
>> @@ -28,9 +28,8 @@ ice_sched_add_root_node(struct ice_port_info *pi,
>>       if (!root)
>>           return -ENOMEM;
>> -    /* coverity[suspicious_sizeof] */
>>       root->children = devm_kcalloc(ice_hw_to_dev(hw), hw->max_children[0],
>> -                      sizeof(*root), GFP_KERNEL);
>> +                      sizeof(*root->children), GFP_KERNEL);
>>       if (!root->children) {
>>           devm_kfree(ice_hw_to_dev(hw), root);
>>           return -ENOMEM;
>> @@ -186,10 +185,9 @@ ice_sched_add_node(struct ice_port_info *pi, u8 
>> layer,
>>       if (!node)
>>           return -ENOMEM;
>>       if (hw->max_children[layer]) {
>> -        /* coverity[suspicious_sizeof] */
>>           node->children = devm_kcalloc(ice_hw_to_dev(hw),
>>                             hw->max_children[layer],
>> -                          sizeof(*node), GFP_KERNEL);
>> +                          sizeof(*node->children), GFP_KERNEL);
>>           if (!node->children) {
>>               devm_kfree(ice_hw_to_dev(hw), node);
>>               return -ENOMEM;
> 
> 
> Kind regards,
> 
> Paul