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] [day] [month] [year] [list]
Message-ID: <20240706095258.GB1481495@kernel.org>
Date: Sat, 6 Jul 2024 10:52:58 +0100
From: Simon Horman <horms@...nel.org>
To: Aleksandr Mishin <amishin@...rgos.ru>
Cc: Anirudh Venkataramanan <anirudh.venkataramanan@...el.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	Tony Nguyen <anthony.l.nguyen@...el.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH net] ice: Adjust memory overrun in
 ice_sched_add_root_node() and ice_sched_add_node()

On Fri, Jul 05, 2024 at 07:36:20PM +0300, Aleksandr Mishin wrote:
> 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 in this calls there are 'sizeof(*root)'
> instead of 'sizeof(root)' and 'sizeof(*node)' instead of 'sizeof(node)'.
> So memory is allocated for structures instead pointers. This lead to
> significant memory overrun.
> Looks like it was done for "coverity[suspicious_sizeof] workaround".

Hi Aleksandr,

While I agree that your patch is correct, I doesn't look to me like it was
done for "coverity[suspicious_sizeof] workaround", as that annotation was
added after the cited patch where the problem appears to have been
introduced.

> 
> Adjust memory overrun by correcting devm_kcalloc() parameters.

I also think it is misleading to describe this as an overrun.  In my
opinion, an overrun refers to writing over the end of a buffer, or similar
conditions where values are written to memory or read from that is not
intended for that purpose.

But that is not the case here.  Instead it is an over allocation of memory.
Which is, in a sense, the opposite of an overrun. I suggest updating the
description of the problem.

> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: dc49c7723676 ("ice: Get MAC/PHY/link info and scheduler topology")

I do agree there is a problem. But I'm not convinced this is fixing a bug -
is the overallocation of memory manifesting, in a real problem, other than
perhaps contributing to memory pressure (I assume in not a very significant
way).

My feeling is that it would be better to target this change to net-next and
drop the Fixes tag.

> Signed-off-by: Aleksandr Mishin <amishin@...rgos.ru>

...

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ