Re: [PATCH v3 net 1/4] net: hns3: fix vector resource leak in hns3_nic_alloc_vector_data
From: Jijie Shao
Date: Wed Nov 04 2026 - 06:59:32 EST
on 2026/11/3 18:18, netdev-bot+sashiko@xxxxxxxxxx wrote:
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit message does not give the real reason for one line of
the fix.
--
net: hns3: fix vector resource leak in hns3_nic_alloc_vector_data
This patch fixes the error path in hns3_nic_alloc_vector_data() for when
devm_kcalloc() of priv->tqp_vector fails. The new path calls put_vector()
for every vector that get_vector() acquired, and resets priv->vector_num
to 0 before returning.
Call put_vector() for every acquired vector before returning the[Severity: Low]
error, and reset priv->vector_num to 0 to avoid a NULL deref in the
subsequent hns3_nic_dealloc_vector_data(), which iterates
priv->tqp_vector (still NULL after the kcalloc failure) using
priv->vector_num.
Is this the right reason for the priv->vector_num reset? As far as I can
tell, no path calls hns3_nic_dealloc_vector_data() after
hns3_nic_alloc_vector_data() fails.
In hns3_client_init(), the failure jumps to a label that comes after the
dealloc call:
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:hns3_client_init() {
...
ret = hns3_nic_alloc_vector_data(priv);
if (ret) {
ret = -ENOMEM;
goto out_alloc_vector_data;
}
...
out_init_vector_data:
hns3_nic_dealloc_vector_data(priv);
out_alloc_vector_data:
...
}
hns3_reset_notify_init_enet() works the same way:
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:hns3_reset_notify_init_enet() {
...
ret = hns3_nic_alloc_vector_data(priv);
if (ret)
goto err_put_ring;
...
err_dealloc_vector:
hns3_nic_dealloc_vector_data(priv);
err_put_ring:
...
}
hns3_client_uninit() and hns3_reset_notify_uninit_enet() both start with
test_and_clear_bit(HNS3_NIC_STATE_INITED, &priv->state). They bail out
before calling hns3_nic_dealloc_vector_data() if the bit is clear.
HNS3_NIC_STATE_INITED is only set once init succeeds, so it is still
clear after this failure. That means the NULL deref described above
cannot happen, with or without this patch.
The reset may still be useful for other code that indexes
priv->tqp_vector by priv->vector_num while the netdev stays alive after
a failed reset re-init. One example is hns3_dump_coal_info() in
hns3_debugfs.c:
for (i = 0; i < priv->vector_num; i++) {
tqp_vector = &priv->tqp_vector[i];
Should the commit message name the code that actually depends on
priv->vector_num being 0, rather than hns3_nic_dealloc_vector_data()?
Acked. The NULL deref in hns3_nic_dealloc_vector_data() cannot
happen because the failure path jumps past it. The real reason
for the reset is to protect code that indexes priv->tqp_vector
by priv->vector_num while the netdev stays alive after a failed
reset re-init (e.g. hns3_dump_coal_info()). Will adjust the
commit message in v4.