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()

From: Paul Menzel
Date: Tue Jul 09 2024 - 04:51:38 EST


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@xxxxxxxxx>
Signed-off-by: Aleksandr Mishin <amishin@xxxxxxxxxx>
---
v3:
- Update comment and use the correct entities as suggested by Przemek
v2: https://lore.kernel.org/all/20240706140518.9214-1-amishin@xxxxxxxxxx/
- 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@xxxxxxxxxx/)
v1: https://lore.kernel.org/all/20240705163620.12429-1-amishin@xxxxxxxxxx/

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