Received-SPF: pass (google.com: domain of linux-kernel+bounces-76008-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <13059273-4b1c-4e8e-a929-25fcf01bb727@infradead.org>
Date: Thu, 22 Feb 2024 16:23:08 +0900
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC net] ps3/gelic: Fix possible NULL pointer dereference
Content-Language: en-US
To: Simon Horman <horms@kernel.org>, "David S. Miller" <davem@davemloft.net>,
 Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>, Nicholas Piggin
 <npiggin@gmail.com>, Christophe Leroy <christophe.leroy@csgroup.eu>,
 "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>,
 "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>, Jeff Garzik <jeff@garzik.org>,
 Dan Carpenter <dan.carpenter@linaro.org>, netdev@vger.kernel.org,
 linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
References: <20240221-ps3-gelic-null-deref-v1-1-f4fe159c7cb0@kernel.org>
From: Geoff Levand <geoff@infradead.org>
In-Reply-To: <20240221-ps3-gelic-null-deref-v1-1-f4fe159c7cb0@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi Simon,

On 2/22/24 01:56, Simon Horman wrote:
> Fix possible NULL pointer dereference in gelic_card_release_tx_chain()
> 
> The cited commit introduced a netdev variable to
> gelic_card_release_tx_chain() which is set unconditionally on each
> iteration of a for loop.
> 
> It is set to the value of tx_chain->tail->skb->dev.  However, in some
> cases it is assumed that tx_chain->tail->skb may be NULL. And if that
> occurs, setting netdev will cause a NULl pointer dereference.
> 
> Given the age of this code I do wonder if this can occur in practice.
> But to be on the safe side this patch assumes that it can and aims to
> avoid the dereference in the case where tx_chain->tail->skb may be NULL.

After 17+ years I never hit this, and never heard of anyone hitting it...

> Flagged by Smatch.
> Compile tested only.

Thanks for 'fixing' this.

Acked-by: Geoff Levand <geoff@infradead.org>