* [PATCH] ibmvnic: fix krealloc() memory leak [not found] <20260526184105.18962-1-grandmaster@al2klimov.de> @ 2026-05-26 18:41 ` Alexander A. Klimov 2026-05-26 20:50 ` Nicolai Buchwitz 0 siblings, 1 reply; 3+ messages in thread From: Alexander A. Klimov @ 2026-05-26 18:41 UTC (permalink / raw) To: 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, open list:IBM Power SRIOV Virtual NIC Device Driver, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), open list Cc: Alexander A. Klimov 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 <grandmaster@al2klimov.de> --- 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; - 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); - if (!adapter->vpd->buff) { + if (!buff) { dev_err(dev, "Could allocate VPD buffer\n"); return -ENOMEM; } + adapter->vpd->buff = buff; adapter->vpd->dma_addr = dma_map_single(dev, adapter->vpd->buff, adapter->vpd->len, -- 2.54.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ibmvnic: fix krealloc() memory leak 2026-05-26 18:41 ` [PATCH] ibmvnic: fix krealloc() memory leak Alexander A. Klimov @ 2026-05-26 20:50 ` Nicolai Buchwitz 2026-05-27 17:22 ` Alexander A. Klimov 0 siblings, 1 reply; 3+ messages in thread From: Nicolai Buchwitz @ 2026-05-26 20:50 UTC (permalink / raw) 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, linuxppc-dev, linux-kernel 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 <grandmaster@al2klimov.de> > --- > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ibmvnic: fix krealloc() memory leak 2026-05-26 20:50 ` Nicolai Buchwitz @ 2026-05-27 17:22 ` Alexander A. Klimov 0 siblings, 0 replies; 3+ messages in thread From: Alexander A. Klimov @ 2026-05-27 17:22 UTC (permalink / raw) To: Nicolai Buchwitz 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, linuxppc-dev, linux-kernel On 5/26/26 22:50, Nicolai Buchwitz wrote: > Hi Alex > > You patch is missing the prefix with the target tree. Please have > a look at [1] for more details on the workflow. Damn. :-) So far I've always got away with whatever I copied from the majority of git log --oneline --follow FILE > > 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 <grandmaster@al2klimov.de> >> --- >> 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, Cool! No leak = no problem = nothing to do here ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-27 17:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260526184105.18962-1-grandmaster@al2klimov.de>
2026-05-26 18:41 ` [PATCH] ibmvnic: fix krealloc() memory leak Alexander A. Klimov
2026-05-26 20:50 ` Nicolai Buchwitz
2026-05-27 17:22 ` Alexander A. Klimov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox