From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.tipi-net.de (mail.tipi-net.de [194.13.80.246]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 222A626C3BD; Tue, 26 May 2026 20:50:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=194.13.80.246 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779828634; cv=none; b=l62os8h7raXqPCHhggfKzE+cf6PPZ0x8migiFIxA1+H+tmpFvUuwEnlk1wOyqElhHKVPXdW2gQNaL7XdDRwlQ7u4l7gfiwIVIGvLMaq8Ez/umNeUbaxneiNOUnH0/Mz9BfC9gxjbJPDwSGwLC3g40+ikK91f0iwkkd7E1o7LDl4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779828634; c=relaxed/simple; bh=3MsbVQMRlkWyRmM+scobE8Chf8BwX3w2hipiqBeSQuE=; h=MIME-Version:Date:From:To:Cc:Subject:In-Reply-To:References: Message-ID:Content-Type; b=tcHCawQ3etM7n0JwXKDjwgkdgpxqnBor9Wcoy++cb9AKFjNxbL5LpQ8s90nsDF1OegbNJOqFZBp3vemvupHJoWbh/Z5zEx25BgF8JMf0mYIdQA9wDDRJUfVAEHnoNdMu7OvFnK8psIFKAEpLmyMvbqhLNTQGBxQf3LMroKuscSw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tipi-net.de; spf=pass smtp.mailfrom=tipi-net.de; dkim=pass (2048-bit key) header.d=tipi-net.de header.i=@tipi-net.de header.b=Rj0TPyz1; arc=none smtp.client-ip=194.13.80.246 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tipi-net.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tipi-net.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tipi-net.de header.i=@tipi-net.de header.b="Rj0TPyz1" Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 82ECBA4B47; Tue, 26 May 2026 22:50:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tipi-net.de; s=dkim; t=1779828627; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=YxIv+z1i7BXKHKGu2iZTSaCZuD3KwsuMfvkF03U6b30=; b=Rj0TPyz16ddOVcu6NGLZel8fDz+TRyxwiP4+JhzkJZbzWYsOSGIZ+zObP76tXE4Vs5QK29 HS9XbmZxA973Zhq+wp7AqhnIAoYUSw5bAJcNkOPsBlrhyMdCJ4AI86DDxYL1GqiA5zhMbj jHtb+DsScJyyz1kBTr1vd5LjhYu1PaFDH8sRCGCSUjkSfhmsKHXAu00wAVEjOKMLvrUlrg gB/SssvHnFNn5m20rHtdf0W3QOrCYChbZYYPwkskWplqnKrS9k0lrXnWrW7Eazy3wwCaEa rReEkauXZOLjZRN1nWhfLRZd30kQPfntTyLJN4DMIT60rpErwd06e/CZ64yv0A== Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Date: Tue, 26 May 2026 22:50:20 +0200 From: Nicolai Buchwitz To: "Alexander A. Klimov" Cc: Haren Myneni , Rick Lindsley , Nick Child , Madhavan Srinivasan , Michael Ellerman , Nicholas Piggin , "Christophe Leroy (CS GROUP)" , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Thomas Falcon , Desnes Augusto Nunes do Rosario , netdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ibmvnic: fix krealloc() memory leak In-Reply-To: <20260526184105.18962-6-grandmaster@al2klimov.de> References: <20260526184105.18962-1-grandmaster@al2klimov.de> <20260526184105.18962-6-grandmaster@al2klimov.de> Message-ID: X-Sender: nb@tipi-net.de Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 Hi Alex You patch is missing the prefix with the target tree. Please have a look at [1] for more details on the workflow. On 26.5.2026 20:41, Alexander A. Klimov wrote: > Don't just overwrite the original pointer passed to krealloc() > with its return value without checking latter: > > MEM = krealloc(MEM, SZ, GFP); > > If krealloc() returns NULL, that erases the pointer > to the still allocated memory, hence leaks this memory. > Instead, use a temporary variable, check it's not NULL > and only then assign it to the original pointer: > > TMP = krealloc(MEM, SZ, GFP); > if (!TMP) return; > MEM = TMP; > > Fixes: 4e6759be28e4 ("ibmvnic: Feature implementation of Vital Product > Data (VPD) for the ibmvnic driver") > Signed-off-by: Alexander A. Klimov > --- > drivers/net/ethernet/ibm/ibmvnic.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c > b/drivers/net/ethernet/ibm/ibmvnic.c > index 5a510eed335e..25d1d844ad19 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -1761,8 +1761,9 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter > *adapter) > union ibmvnic_crq crq; > int len = 0; > int rc; > + unsigned char *buff = adapter->vpd->buff; Should be reverse x-mas tree (longest to shortest). > > - if (adapter->vpd->buff) > + if (buff) > len = adapter->vpd->len; > > mutex_lock(&adapter->fw_lock); > @@ -1788,17 +1789,17 @@ static int ibmvnic_get_vpd(struct > ibmvnic_adapter *adapter) > if (!adapter->vpd->len) > return -ENODATA; > > - if (!adapter->vpd->buff) > - adapter->vpd->buff = kzalloc(adapter->vpd->len, GFP_KERNEL); > + if (!buff) > + buff = kzalloc(adapter->vpd->len, GFP_KERNEL); > else if (adapter->vpd->len != len) > - adapter->vpd->buff = > - krealloc(adapter->vpd->buff, > - adapter->vpd->len, GFP_KERNEL); > + buff = krealloc(buff, > + adapter->vpd->len, GFP_KERNEL); Dead branch? The only caller, init_resources(), kzalloc()s a fresh vpd right before, and resets run release_vpd_data() first, so vpd->buff is always NULL here and kzalloc() above always wins. The leak can't trigger, which makes the Fixes tag misleading. > > - if (!adapter->vpd->buff) { > + if (!buff) { > dev_err(dev, "Could allocate VPD buffer\n"); > return -ENOMEM; > } > + adapter->vpd->buff = buff; If you keep it anyway: on failure the old buffer stays in vpd->buff while vpd->len is already the new size, a mismatch the original avoided by NULLing. kfree() it (krealloc() does not free on failure) and NULL before -ENOMEM. > > adapter->vpd->dma_addr = > dma_map_single(dev, adapter->vpd->buff, adapter->vpd->len, [1] https://docs.kernel.org/process/maintainer-netdev.html Thanks, Nicolai