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: <20241105122516.GA4507@kernel.org>
Date: Tue, 5 Nov 2024 12:25:16 +0000
From: Simon Horman <horms@...nel.org>
To: Shinas Rasheed <srasheed@...vell.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, hgani@...vell.com,
	sedara@...vell.com, vimleshk@...vell.com, wizhao@...hat.com,
	kongyen@...hat.com, pabeni@...hat.com, kheib@...hat.com,
	mschmidt@...hat.com, Veerasenareddy Burru <vburru@...vell.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Satananda Burla <sburla@...vell.com>,
	Abhijit Ayarekar <aayarekar@...vell.com>
Subject: Re: [PATCH net v1 2/3] octeon_ep: add checks to fix NULL pointer
 dereferences

On Fri, Nov 01, 2024 at 03:34:14AM -0700, Shinas Rasheed wrote:
> Add Checks to avoid NULL pointer references that might
> happen in rare and corner cases
> 
> Fixes: 6a610a46bad1 ("octeon_ep: add support for ndo ops")
> Fixes: 1f2c2d0cee02 ("octeon_ep: add hardware configuration APIs")
> Fixes: 0807dc76f3bf ("octeon_ep: support Octeon CN10K devices")

Hi Shinas,

As this has both three Fixes tags and three hunks, I suspect
it is fixing three separate but similar problems. And if so,
would be best split into three patches, one patch per problem.

Further, as an overall comment for the entire series, I think more
explanation of how these problems can arise is needed. Are they race
conditions, artifacts of tear-down or error handling, ... And what
execution paths lead to them? Stack traces, if available, would also be
useful to include.

> Signed-off-by: Shinas Rasheed <srasheed@...vell.com>
> ---
>  drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c | 9 ++++++++-
>  drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c | 9 ++++++++-
>  drivers/net/ethernet/marvell/octeon_ep/octep_main.c    | 3 +++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> index b5805969404f..b87336b2e4b9 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
> @@ -617,7 +617,14 @@ static irqreturn_t octep_rsvd_intr_handler_cn93_pf(void *dev)
>  static irqreturn_t octep_ioq_intr_handler_cn93_pf(void *data)
>  {
>  	struct octep_ioq_vector *vector = (struct octep_ioq_vector *)data;
> -	struct octep_oq *oq = vector->oq;
> +	struct octep_oq *oq;
> +
> +	if (!vector)
> +		return IRQ_HANDLED;
> +	oq = vector->oq;
> +
> +	if (!oq || !(oq->napi))

nit: I don't think you need parentheses around op->napi. Likeiwse in patch 3/3.

> +		return IRQ_HANDLED;
>  
>  	napi_schedule_irqoff(oq->napi);
>  	return IRQ_HANDLED;

...

-- 
pw-bot: changes-requested

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ