Re: [PATCH v2 13/14] hv: netvsc: handle a NULL request address on empty completions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] netvsc: Type confusion and memory corruption via untrusted host msglen mismatch
- [Low] netvsc: data race on global wake_queue ethtool statistic
--

commit cc27953d7c3e5380170944c920266d1ad5b66616
Author: Emerson Busson <emersonbusson@gmail.com>
Subject: hv: netvsc: handle a NULL request address on empty completions

This commit introduces a new check in the empty-payload branch of the netvsc
send completion path to tolerate NULL request addresses, which occur during
control RNDIS sends. It extracts the queue wake and accounting logic into a
new helper function and provides associated KUnit tests.

[Severity: Critical]
In drivers/net/hyperv/netvsc.c, within the netvsc_send_completion and
netvsc_send_tx_complete functions, is there a potential type confusion
regression? The empty-payload branch relies on the untrusted host msglen == 0
to cast the context pointer to an nvsp_message. If the pointer is actually an
sk_buff, does this result in a memory leak? Conversely, if the host sends a
msglen > 0 for a channel_init_pkt transaction, the pointer is cast to an
sk_buff in netvsc_send_tx_complete. Since the pointer is an interior struct
member rather than an sk_buff, could this cause out-of-bounds reads and
slab memory corruption when napi_consume_skb is called on it? This sequence
is reachable by an untrusted host, which violates the security boundary in
confidential virtual machine environments where the host is untrusted.

[Severity: Low]
In drivers/net/hyperv/netvsc.c, at line 788 within the newly extracted
netvsc_send_acct function, does this code introduce a data race on the
wake_queue ethtool statistic? The increment operation on
ndev_ctx->eth_stats.wake_queue is performed non-atomically. When multiple
VMBus channels process completions in concurrent NAPI softirq contexts on
different CPUs, and both channels need to wake their respective stopped
transmit queues, could they simultaneously evaluate the queue state and
update the shared global counter without synchronization, resulting in lost
increments?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007190752.336426-1-emersonbusson@gmail.com?part=13