* Re: [PATCH net-next] net: mana: hardening: Reject zero max_num_queues from MANA_QUERY_VPORT_CONFIG
2026-03-26 17:48 [PATCH net-next] net: mana: hardening: Reject zero max_num_queues from MANA_QUERY_VPORT_CONFIG Erni Sri Satya Vennela
@ 2026-03-31 9:33 ` Paolo Abeni
2026-03-31 18:02 ` Erni Sri Satya Vennela
2026-04-10 5:16 ` Erni Sri Satya Vennela
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2026-03-31 9:33 UTC (permalink / raw)
To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, longli,
andrew+netdev, davem, edumazet, kuba, ssengar, dipayanroy,
gargaditya, shirazsaleem, kees, linux-hyperv, netdev,
linux-kernel
On 3/26/26 6:48 PM, Erni Sri Satya Vennela wrote:
> As a part of MANA hardening for CVM, validate that max_num_sq and
> max_num_rq returned by MANA_QUERY_VPORT_CONFIG are not zero. These
> values flow into apc->num_queues, which is used as an allocation count
> and loop bound. A zero value would result in zero-size allocations and
> incorrect driver behavior.
>
> Return -EPROTO if either value is zero.
>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b39e8b920791..a4197b4b0597 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1249,6 +1249,12 @@ static int mana_query_vport_cfg(struct mana_port_context *apc, u32 vport_index,
>
> *max_sq = resp.max_num_sq;
> *max_rq = resp.max_num_rq;
> +
> + if (*max_sq == 0 || *max_rq == 0) {
> + netdev_err(apc->ndev, "Invalid max queues from vPort config\n");
> + return -EPROTO;
AI review says:
Will returning -EPROTO here expose a pre-existing resource leak in the
driver's teardown path?
If mana_query_vport_cfg() returns an error, mana_init_port() fails and
mana_probe_port() frees the ndev, leaving ac->ports[i] as NULL. In
mana_probe(), the port initialization loop breaks upon this error, but
the err variable is then overwritten:
mana_probe() {
...
for (i = 0; i < ac->num_ports; i++) {
err = mana_probe_port(ac, i, &ac->ports[i]);
if (err) {
dev_err(dev, "Probe Failed for port %d\n", i);
break;
}
}
err = add_adev(gd, "eth");
...
}
If add_adev() succeeds, mana_probe() completes successfully instead of
failing, masking the earlier error while leaving ac->ports[0] as NULL.
Later, when the driver is unloaded or if add_adev() fails and triggers
immediate cleanup, mana_remove() is called. It iterates over ac->ports
and, upon encountering the NULL device, immediately executes goto out:
mana_remove() {
...
for (i = 0; i < ac->num_ports; i++) {
ndev = ac->ports[i];
if (!ndev) {
if (i == 0)
...
goto out;
}
...
}
mana_destroy_eq(ac);
out:
...
}
Because the out label in mana_remove() is located after the
mana_destroy_eq(ac) call, jumping there completely skips destroying the
event queues allocated earlier by mana_create_eq(ac).
In a Confidential Virtual Machine context, could an untrusted hypervisor
repeatedly return invalid configs to continuously leak guest memory and
hardware queues?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next] net: mana: hardening: Reject zero max_num_queues from MANA_QUERY_VPORT_CONFIG
2026-03-26 17:48 [PATCH net-next] net: mana: hardening: Reject zero max_num_queues from MANA_QUERY_VPORT_CONFIG Erni Sri Satya Vennela
2026-03-31 9:33 ` Paolo Abeni
@ 2026-04-10 5:16 ` Erni Sri Satya Vennela
1 sibling, 0 replies; 4+ messages in thread
From: Erni Sri Satya Vennela @ 2026-04-10 5:16 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, ssengar, dipayanroy, gargaditya,
shirazsaleem, kees, linux-hyperv, netdev, linux-kernel
On Thu, Mar 26, 2026 at 10:48:10AM -0700, Erni Sri Satya Vennela wrote:
> As a part of MANA hardening for CVM, validate that max_num_sq and
> max_num_rq returned by MANA_QUERY_VPORT_CONFIG are not zero. These
> values flow into apc->num_queues, which is used as an allocation count
> and loop bound. A zero value would result in zero-size allocations and
> incorrect driver behavior.
>
> Return -EPROTO if either value is zero.
>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b39e8b920791..a4197b4b0597 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1249,6 +1249,12 @@ static int mana_query_vport_cfg(struct mana_port_context *apc, u32 vport_index,
>
> *max_sq = resp.max_num_sq;
> *max_rq = resp.max_num_rq;
> +
> + if (*max_sq == 0 || *max_rq == 0) {
> + netdev_err(apc->ndev, "Invalid max queues from vPort config\n");
> + return -EPROTO;
> + }
> +
> if (resp.num_indirection_ent > 0 &&
> resp.num_indirection_ent <= MANA_INDIRECT_TABLE_MAX_SIZE &&
> is_power_of_2(resp.num_indirection_ent)) {
> --
> 2.34.1
Hi,
Gentle reminder regarding this patch.
I would really appreciate any feedback whenever you get a chance.
Please let me know if any changes are required from my side.
Thanks for your time.
Regards,
Vennela
^ permalink raw reply [flat|nested] 4+ messages in thread