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 <[email protected]>
---
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
