mbox series

[net-next,0/3] virtchnl: fix fake 1-elem arrays

Message ID 20230728155207.10042-1-aleksander.lobakin@intel.com
Headers
Series virtchnl: fix fake 1-elem arrays |

Message

Alexander Lobakin July 28, 2023, 3:52 p.m. UTC 
  6.5-rc1 started spitting warning splats when composing virtchnl
messages, precisely on virtchnl_rss_key and virtchnl_lut:

[   84.167709] memcpy: detected field-spanning write (size 52) of single
field "vrk->key" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1095
(size 1)
[   84.169915] WARNING: CPU: 3 PID: 11 at drivers/net/ethernet/intel/
iavf/iavf_virtchnl.c:1095 iavf_set_rss_key+0x123/0x140 [iavf]
...
[   84.191982] Call Trace:
[   84.192439]  <TASK>
[   84.192900]  ? __warn+0xc9/0x1a0
[   84.193353]  ? iavf_set_rss_key+0x123/0x140 [iavf]
[   84.193818]  ? report_bug+0x12c/0x1b0
[   84.194266]  ? handle_bug+0x42/0x70
[   84.194714]  ? exc_invalid_op+0x1a/0x50
[   84.195149]  ? asm_exc_invalid_op+0x1a/0x20
[   84.195592]  ? iavf_set_rss_key+0x123/0x140 [iavf]
[   84.196033]  iavf_watchdog_task+0xb0c/0xe00 [iavf]
...
[   84.225476] memcpy: detected field-spanning write (size 64) of single
field "vrl->lut" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1127
(size 1)
[   84.227190] WARNING: CPU: 27 PID: 1044 at drivers/net/ethernet/intel/
iavf/iavf_virtchnl.c:1127 iavf_set_rss_lut+0x123/0x140 [iavf]
...
[   84.246601] Call Trace:
[   84.247228]  <TASK>
[   84.247840]  ? __warn+0xc9/0x1a0
[   84.248263]  ? iavf_set_rss_lut+0x123/0x140 [iavf]
[   84.248698]  ? report_bug+0x12c/0x1b0
[   84.249122]  ? handle_bug+0x42/0x70
[   84.249549]  ? exc_invalid_op+0x1a/0x50
[   84.249970]  ? asm_exc_invalid_op+0x1a/0x20
[   84.250390]  ? iavf_set_rss_lut+0x123/0x140 [iavf]
[   84.250820]  iavf_watchdog_task+0xb16/0xe00 [iavf]

Gustavo already tried to fix those back in 2021[0][1]. Unfortunately,
a VM can run a different kernel than the host, meaning that those
structures are sorta ABI.
However, it is possible to have proper flex arrays + struct_size()
calculations and still send the very same messages with the same sizes.
The common rule is:

elem[1] -> elem[]
size = struct_size() + <difference between the old and the new msg size>

The "old" size in the current code is calculated 3 different ways for
10 virtchnl structures total. Each commit addresses one of the ways
cumulatively instead of per-structure.

I was planning to send it to -net initially, but given that virtchnl was
renamed from i40evf and got some fat style cleanup commits in the past,
it's not very straightforward to even pick appropriate SHAs, not
speaking of automatic portability. I may send manual backports for
a couple of the latest supported kernels later on if anyone needs it
at all.

[0] https://lore.kernel.org/all/20210525230912.GA175802@embeddedor
[1] https://lore.kernel.org/all/20210525231851.GA176647@embeddedor

Alexander Lobakin (3):
  virtchnl: fix fake 1-elem arrays in structs allocated as `nents + 1` -
    1
  virtchnl: fix fake 1-elem arrays in structures allocated as `nents +
    1`
  virtchnl: fix fake 1-elem arrays for structures allocated as `nents`

 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    |   9 +-
 drivers/net/ethernet/intel/iavf/iavf.h        |   6 +-
 drivers/net/ethernet/intel/iavf/iavf_client.c |   4 +-
 drivers/net/ethernet/intel/iavf/iavf_client.h |   2 +-
 .../net/ethernet/intel/iavf/iavf_virtchnl.c   |  75 +++++------
 drivers/net/ethernet/intel/ice/ice_virtchnl.c |   2 +-
 include/linux/avf/virtchnl.h                  | 127 +++++++++++-------
 7 files changed, 124 insertions(+), 101 deletions(-)

Comments

Alexander Lobakin Aug. 3, 2023, 3:55 p.m. UTC | #1 
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: Fri, 28 Jul 2023 17:52:04 +0200

> 6.5-rc1 started spitting warning splats when composing virtchnl
> messages, precisely on virtchnl_rss_key and virtchnl_lut:
> 
> [   84.167709] memcpy: detected field-spanning write (size 52) of single
> field "vrk->key" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1095
> (size 1)
> [   84.169915] WARNING: CPU: 3 PID: 11 at drivers/net/ethernet/intel/
> iavf/iavf_virtchnl.c:1095 iavf_set_rss_key+0x123/0x140 [iavf]

(tender ping :D)

Thanks,
Olek
Alexander Lobakin Aug. 4, 2023, 4:38 p.m. UTC | #2 
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: Fri, 28 Jul 2023 17:52:04 +0200

> 6.5-rc1 started spitting warning splats when composing virtchnl
> messages, precisely on virtchnl_rss_key and virtchnl_lut:
> 
> [   84.167709] memcpy: detected field-spanning write (size 52) of single
> field "vrk->key" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1095
> (size 1)
> [   84.169915] WARNING: CPU: 3 PID: 11 at drivers/net/ethernet/intel/
> iavf/iavf_virtchnl.c:1095 iavf_set_rss_key+0x123/0x140 [iavf]

[...]

>  .../ethernet/intel/i40e/i40e_virtchnl_pf.c    |   9 +-
>  drivers/net/ethernet/intel/iavf/iavf.h        |   6 +-
>  drivers/net/ethernet/intel/iavf/iavf_client.c |   4 +-
>  drivers/net/ethernet/intel/iavf/iavf_client.h |   2 +-
>  .../net/ethernet/intel/iavf/iavf_virtchnl.c   |  75 +++++------
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c |   2 +-
>  include/linux/avf/virtchnl.h                  | 127 +++++++++++-------
>  7 files changed, 124 insertions(+), 101 deletions(-)
> 

Tony, could you please take it via your next tree? I'd like the
validation to make sure more different host <-> guest pairs work.

(with Kees' tags, assuming he reviewed and approved the whole series, I
 asked about #2 already)

Thanks,
Olek
Tony Nguyen Aug. 4, 2023, 6:07 p.m. UTC | #3 
On 8/4/2023 9:38 AM, Alexander Lobakin wrote:
> From: Alexander Lobakin <aleksander.lobakin@intel.com>
> Date: Fri, 28 Jul 2023 17:52:04 +0200
> 
>> 6.5-rc1 started spitting warning splats when composing virtchnl
>> messages, precisely on virtchnl_rss_key and virtchnl_lut:
>>
>> [   84.167709] memcpy: detected field-spanning write (size 52) of single
>> field "vrk->key" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1095
>> (size 1)
>> [   84.169915] WARNING: CPU: 3 PID: 11 at drivers/net/ethernet/intel/
>> iavf/iavf_virtchnl.c:1095 iavf_set_rss_key+0x123/0x140 [iavf]
> 
> [...]
> 
>>   .../ethernet/intel/i40e/i40e_virtchnl_pf.c    |   9 +-
>>   drivers/net/ethernet/intel/iavf/iavf.h        |   6 +-
>>   drivers/net/ethernet/intel/iavf/iavf_client.c |   4 +-
>>   drivers/net/ethernet/intel/iavf/iavf_client.h |   2 +-
>>   .../net/ethernet/intel/iavf/iavf_virtchnl.c   |  75 +++++------
>>   drivers/net/ethernet/intel/ice/ice_virtchnl.c |   2 +-
>>   include/linux/avf/virtchnl.h                  | 127 +++++++++++-------
>>   7 files changed, 124 insertions(+), 101 deletions(-)
>>
> 
> Tony, could you please take it via your next tree? I'd like the
> validation to make sure more different host <-> guest pairs work.
> 
> (with Kees' tags, assuming he reviewed and approved the whole series, I
>   asked about #2 already)
> 
> Thanks,
> Olek

Ok, will apply it today. For the future if you want it through IWL, can 
you tag it with the iwl-* target (and have IWL in the To)? Since this 
had 'net-next' and was 'To' netdev maintainers, I took it that you 
wanted it taken through netdev.

Thanks,
Tony
Alexander Lobakin Aug. 4, 2023, 6:09 p.m. UTC | #4 
From: Tony Nguyen <anthony.l.nguyen@intel.com>
Date: Fri, 4 Aug 2023 11:07:14 -0700

> On 8/4/2023 9:38 AM, Alexander Lobakin wrote:
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Date: Fri, 28 Jul 2023 17:52:04 +0200
>>
>>> 6.5-rc1 started spitting warning splats when composing virtchnl
>>> messages, precisely on virtchnl_rss_key and virtchnl_lut:
>>>
>>> [   84.167709] memcpy: detected field-spanning write (size 52) of single
>>> field "vrk->key" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1095
>>> (size 1)
>>> [   84.169915] WARNING: CPU: 3 PID: 11 at drivers/net/ethernet/intel/
>>> iavf/iavf_virtchnl.c:1095 iavf_set_rss_key+0x123/0x140 [iavf]
>>
>> [...]
>>
>>>   .../ethernet/intel/i40e/i40e_virtchnl_pf.c    |   9 +-
>>>   drivers/net/ethernet/intel/iavf/iavf.h        |   6 +-
>>>   drivers/net/ethernet/intel/iavf/iavf_client.c |   4 +-
>>>   drivers/net/ethernet/intel/iavf/iavf_client.h |   2 +-
>>>   .../net/ethernet/intel/iavf/iavf_virtchnl.c   |  75 +++++------
>>>   drivers/net/ethernet/intel/ice/ice_virtchnl.c |   2 +-
>>>   include/linux/avf/virtchnl.h                  | 127 +++++++++++-------
>>>   7 files changed, 124 insertions(+), 101 deletions(-)
>>>
>>
>> Tony, could you please take it via your next tree? I'd like the
>> validation to make sure more different host <-> guest pairs work.
>>
>> (with Kees' tags, assuming he reviewed and approved the whole series, I
>>   asked about #2 already)
>>
>> Thanks,
>> Olek
> 
> Ok, will apply it today. For the future if you want it through IWL, can

Great, thanks!

> you tag it with the iwl-* target (and have IWL in the To)? Since this
> had 'net-next' and was 'To' netdev maintainers, I took it that you
> wanted it taken through netdev.

Sure, I know, just for some reason targeted this directly to net at
first, but then realized it would be better for this to go via IWL
:clownface:

> 
> Thanks,
> Tony

Thanks,
Olek