[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <916a6c1a-681e-4e5e-8d49-75d0de5c46a1@redhat.com>
Date: Tue, 13 Jan 2026 16:06:05 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>, Jakub Kicinski <kuba@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
Jason Wang <jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Eugenio Pérez <eperezma@...hat.com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Akihiko Odaki <akihiko.odaki@...nix.com>, Simon Horman <horms@...nel.org>,
netdev@...r.kernel.org, virtualization@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
Kees Cook <kees@...nel.org>
Subject: Re: [PATCH v2][next] virtio_net: Fix misalignment bug in struct
virtnet_info
On 1/13/26 3:39 PM, Michael S. Tsirkin wrote:
> On Tue, Jan 13, 2026 at 03:30:00PM +0100, Paolo Abeni wrote:
>> On 1/10/26 9:07 AM, Gustavo A. R. Silva wrote:
>>> Use the new TRAILING_OVERLAP() helper to fix a misalignment bug
>>> along with the following warning:
>>>
>>> drivers/net/virtio_net.c:429:46: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>>
>>> This helper creates a union between a flexible-array member (FAM)
>>> and a set of members that would otherwise follow it (in this case
>>> `u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];`). This
>>> overlays the trailing members (rss_hash_key_data) onto the FAM
>>> (hash_key_data) while keeping the FAM and the start of MEMBERS aligned.
>>> The static_assert() ensures this alignment remains.
>>>
>>> Notice that due to tail padding in flexible `struct
>>> virtio_net_rss_config_trailer`, `rss_trailer.hash_key_data`
>>> (at offset 83 in struct virtnet_info) and `rss_hash_key_data` (at
>>> offset 84 in struct virtnet_info) are misaligned by one byte. See
>>> below:
>>>
>>> struct virtio_net_rss_config_trailer {
>>> __le16 max_tx_vq; /* 0 2 */
>>> __u8 hash_key_length; /* 2 1 */
>>> __u8 hash_key_data[]; /* 3 0 */
>>>
>>> /* size: 4, cachelines: 1, members: 3 */
>>> /* padding: 1 */
>>> /* last cacheline: 4 bytes */
>>> };
>>>
>>> struct virtnet_info {
>>> ...
>>> struct virtio_net_rss_config_trailer rss_trailer; /* 80 4 */
>>>
>>> /* XXX last struct has 1 byte of padding */
>>>
>>> u8 rss_hash_key_data[40]; /* 84 40 */
>>> ...
>>> /* size: 832, cachelines: 13, members: 48 */
>>> /* sum members: 801, holes: 8, sum holes: 31 */
>>> /* paddings: 2, sum paddings: 5 */
>>> };
>>>
>>> After changes, those members are correctly aligned at offset 795:
>>>
>>> struct virtnet_info {
>>> ...
>>> union {
>>> struct virtio_net_rss_config_trailer rss_trailer; /* 792 4 */
>>> struct {
>>> unsigned char __offset_to_hash_key_data[3]; /* 792 3 */
>>> u8 rss_hash_key_data[40]; /* 795 40 */
>>> }; /* 792 43 */
>>> }; /* 792 44 */
>>> ...
>>> /* size: 840, cachelines: 14, members: 47 */
>>> /* sum members: 801, holes: 8, sum holes: 35 */
>>> /* padding: 4 */
>>> /* paddings: 1, sum paddings: 4 */
>>> /* last cacheline: 8 bytes */
>>> };
>>>
>>> As a result, the RSS key passed to the device is shifted by 1
>>> byte: the last byte is cut off, and instead a (possibly
>>> uninitialized) byte is added at the beginning.
>>>
>>> As a last note `struct virtio_net_rss_config_hdr *rss_hdr;` is also
>>> moved to the end, since it seems those three members should stick
>>> around together. :)
>>>
>>> Cc: stable@...r.kernel.org
>>> Fixes: ed3100e90d0d ("virtio_net: Use new RSS config structs")
>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>>> ---
>>> Changes in v2:
>>> - Update subject and changelog text (include feedback from Simon and
>>> Michael --thanks folks)
>>> - Add Fixes tag and CC -stable.
>>
>> @Michael, @Jason: This is still apparently targeting 'net-next', but I
>> think it should land in the 'net' tree, right?
>>
>> /P
>
> Probably but I'm yet to properly review it. The thing that puzzles me at
> a first glance is how are things working right now then?
Apparently they aren't ?!?
rss self-tests for virtio_net are failing:
https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/471521/15-rss-api-py/stdout
but the result is into the CI reported as success (no idea why?!?)
/P
Powered by blists - more mailing lists