netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] hv_netvsc: Replace one-element array with flexible array member
@ 2025-01-16 21:19 Thorsten Blum
  2025-01-16 21:39 ` Roman Kisel
  2025-01-18  3:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Thorsten Blum @ 2025-01-16 21:19 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-hardening, Roman Kisel, Thorsten Blum, linux-hyperv, netdev,
	linux-kernel

Replace the deprecated one-element array with a modern flexible array
member in the struct nvsp_1_message_send_receive_buffer_complete.

Use struct_size_t(,,1) instead of sizeof() to maintain the same size.

Compile-tested only.

Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v2:
- Replace sizeof() with struct_size_t(,,1) to maintain the same size
  after feedback from Roman Kisel (thanks!)
- Link to v1: https://lore.kernel.org/r/20250116201635.47870-2-thorsten.blum@linux.dev/
---
 drivers/net/hyperv/hyperv_net.h | 2 +-
 drivers/net/hyperv/netvsc.c     | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index e690b95b1bbb..234db693cefa 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -464,7 +464,7 @@ struct nvsp_1_message_send_receive_buffer_complete {
 	 *  LargeOffset                            SmallOffset
 	 */
 
-	struct nvsp_1_receive_buffer_section sections[1];
+	struct nvsp_1_receive_buffer_section sections[];
 } __packed;
 
 /*
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 9afb08dbc350..d6f5b9ea3109 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -866,7 +866,8 @@ static void netvsc_send_completion(struct net_device *ndev,
 
 	case NVSP_MSG1_TYPE_SEND_RECV_BUF_COMPLETE:
 		if (msglen < sizeof(struct nvsp_message_header) +
-				sizeof(struct nvsp_1_message_send_receive_buffer_complete)) {
+				struct_size_t(struct nvsp_1_message_send_receive_buffer_complete,
+					      sections, 1)) {
 			netdev_err(ndev, "nvsp_msg1 length too small: %u\n",
 				   msglen);
 			return;
-- 
2.48.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] hv_netvsc: Replace one-element array with flexible array member
  2025-01-16 21:19 [PATCH v2] hv_netvsc: Replace one-element array with flexible array member Thorsten Blum
@ 2025-01-16 21:39 ` Roman Kisel
  2025-01-17  0:17   ` Jakub Kicinski
  2025-01-18  3:50 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 6+ messages in thread
From: Roman Kisel @ 2025-01-16 21:39 UTC (permalink / raw)
  To: Thorsten Blum, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: linux-hardening, linux-hyperv, netdev, linux-kernel



On 1/16/2025 1:19 PM, Thorsten Blum wrote:
> Replace the deprecated one-element array with a modern flexible array
> member in the struct nvsp_1_message_send_receive_buffer_complete.
> 
> Use struct_size_t(,,1) instead of sizeof() to maintain the same size.

Thanks!

> 
> Compile-tested only.

The code change looks good to me now. I'll build a VM with this change
to clear my conscience (maybe the change triggers a compiler bug,
however unlikely that sounds) before replying with any tags. Likely next
Monday, but feel free to beat me to it, or perhaps, someone else will
tag this as reviewed by them and/or tested by them.

> 
> Link: https://github.com/KSPP/linux/issues/79
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v2:
> - Replace sizeof() with struct_size_t(,,1) to maintain the same size
>    after feedback from Roman Kisel (thanks!)
> - Link to v1: https://lore.kernel.org/r/20250116201635.47870-2-thorsten.blum@linux.dev/
> ---
>   drivers/net/hyperv/hyperv_net.h | 2 +-
>   drivers/net/hyperv/netvsc.c     | 3 ++-
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index e690b95b1bbb..234db693cefa 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -464,7 +464,7 @@ struct nvsp_1_message_send_receive_buffer_complete {
>   	 *  LargeOffset                            SmallOffset
>   	 */
>   
> -	struct nvsp_1_receive_buffer_section sections[1];
> +	struct nvsp_1_receive_buffer_section sections[];
>   } __packed;
>   
>   /*
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 9afb08dbc350..d6f5b9ea3109 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -866,7 +866,8 @@ static void netvsc_send_completion(struct net_device *ndev,
>   
>   	case NVSP_MSG1_TYPE_SEND_RECV_BUF_COMPLETE:
>   		if (msglen < sizeof(struct nvsp_message_header) +
> -				sizeof(struct nvsp_1_message_send_receive_buffer_complete)) {
> +				struct_size_t(struct nvsp_1_message_send_receive_buffer_complete,
> +					      sections, 1)) {
>   			netdev_err(ndev, "nvsp_msg1 length too small: %u\n",
>   				   msglen);
>   			return;

-- 
Thank you,
Roman


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] hv_netvsc: Replace one-element array with flexible array member
  2025-01-16 21:39 ` Roman Kisel
@ 2025-01-17  0:17   ` Jakub Kicinski
  2025-01-17 11:10     ` Przemek Kitszel
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2025-01-17  0:17 UTC (permalink / raw)
  To: Roman Kisel
  Cc: Thorsten Blum, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, linux-hardening, linux-hyperv, netdev, linux-kernel

On Thu, 16 Jan 2025 13:39:52 -0800 Roman Kisel wrote:
> On 1/16/2025 1:19 PM, Thorsten Blum wrote:
> > Replace the deprecated one-element array with a modern flexible array
> > member in the struct nvsp_1_message_send_receive_buffer_complete.
> > 
> > Use struct_size_t(,,1) instead of sizeof() to maintain the same size.  
> 
> Thanks!
> 
> > 
> > Compile-tested only.  
> 
> The code change looks good to me now. I'll build a VM with this change
> to clear my conscience (maybe the change triggers a compiler bug,
> however unlikely that sounds) before replying with any tags. Likely next
> Monday, but feel free to beat me to it, or perhaps, someone else will
> tag this as reviewed by them and/or tested by them.

Doesn't look like a real bug fix, so would be good to get some signal
soon. The merge window is soon so we'll likely close the trees very
very soon ..

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] hv_netvsc: Replace one-element array with flexible array member
  2025-01-17  0:17   ` Jakub Kicinski
@ 2025-01-17 11:10     ` Przemek Kitszel
  2025-01-17 19:14       ` Roman Kisel
  0 siblings, 1 reply; 6+ messages in thread
From: Przemek Kitszel @ 2025-01-17 11:10 UTC (permalink / raw)
  To: Jakub Kicinski, Roman Kisel
  Cc: Thorsten Blum, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, linux-hardening, linux-hyperv, netdev, linux-kernel

On 1/17/25 01:17, Jakub Kicinski wrote:
> On Thu, 16 Jan 2025 13:39:52 -0800 Roman Kisel wrote:
>> On 1/16/2025 1:19 PM, Thorsten Blum wrote:
>>> Replace the deprecated one-element array with a modern flexible array
>>> member in the struct nvsp_1_message_send_receive_buffer_complete.
>>>
>>> Use struct_size_t(,,1) instead of sizeof() to maintain the same size.

I would add a struct-specific macro or at least put the info into this
struct' doc, that "there is legacy API requirement to allocate at least
one element for the flex array member".

>>
>> Thanks!
>>
>>>
>>> Compile-tested only.
>>
>> The code change looks good to me now. I'll build a VM with this change
>> to clear my conscience (maybe the change triggers a compiler bug,
>> however unlikely that sounds) before replying with any tags. Likely next
>> Monday, but feel free to beat me to it, or perhaps, someone else will
>> tag this as reviewed by them and/or tested by them.
> 
> Doesn't look like a real bug fix, so would be good to get some signal

If the actual usage would be with more than 1 element UBSAN will
complain.

> soon. The merge window is soon so we'll likely close the trees very
> very soon ..
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] hv_netvsc: Replace one-element array with flexible array member
  2025-01-17 11:10     ` Przemek Kitszel
@ 2025-01-17 19:14       ` Roman Kisel
  0 siblings, 0 replies; 6+ messages in thread
From: Roman Kisel @ 2025-01-17 19:14 UTC (permalink / raw)
  To: Przemek Kitszel, Jakub Kicinski
  Cc: Thorsten Blum, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, linux-hardening, linux-hyperv, netdev, linux-kernel



On 1/17/2025 3:10 AM, Przemek Kitszel wrote:
> On 1/17/25 01:17, Jakub Kicinski wrote:
>> On Thu, 16 Jan 2025 13:39:52 -0800 Roman Kisel wrote:
>>> On 1/16/2025 1:19 PM, Thorsten Blum wrote:
>>>> Replace the deprecated one-element array with a modern flexible array
>>>> member in the struct nvsp_1_message_send_receive_buffer_complete.
>>>>
>>>> Use struct_size_t(,,1) instead of sizeof() to maintain the same size.
> 
> I would add a struct-specific macro or at least put the info into this
> struct' doc, that "there is legacy API requirement to allocate at least
> one element for the flex array member".
> 
Perhaps add "No functional changes", too.

Worked for me.

Tested-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Roman Kisel <romank@linux.microsoft.com>

>>>
>>> Thanks!
>>>
>>>>
>>>> Compile-tested only.
>>>
>>> The code change looks good to me now. I'll build a VM with this change
>>> to clear my conscience (maybe the change triggers a compiler bug,
>>> however unlikely that sounds) before replying with any tags. Likely next
>>> Monday, but feel free to beat me to it, or perhaps, someone else will
>>> tag this as reviewed by them and/or tested by them.
>>
>> Doesn't look like a real bug fix, so would be good to get some signal
> 
> If the actual usage would be with more than 1 element UBSAN will
> complain.
Great point!

> 
>> soon. The merge window is soon so we'll likely close the trees very
>> very soon ..
>>
> 

-- 
Thank you,
Roman


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] hv_netvsc: Replace one-element array with flexible array member
  2025-01-16 21:19 [PATCH v2] hv_netvsc: Replace one-element array with flexible array member Thorsten Blum
  2025-01-16 21:39 ` Roman Kisel
@ 2025-01-18  3:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-18  3:50 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
	kuba, pabeni, linux-hardening, romank, linux-hyperv, netdev,
	linux-kernel

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 16 Jan 2025 22:19:32 +0100 you wrote:
> Replace the deprecated one-element array with a modern flexible array
> member in the struct nvsp_1_message_send_receive_buffer_complete.
> 
> Use struct_size_t(,,1) instead of sizeof() to maintain the same size.
> 
> Compile-tested only.
> 
> [...]

Here is the summary with links:
  - [v2] hv_netvsc: Replace one-element array with flexible array member
    https://git.kernel.org/netdev/net-next/c/3df22e751027

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-01-18  3:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-16 21:19 [PATCH v2] hv_netvsc: Replace one-element array with flexible array member Thorsten Blum
2025-01-16 21:39 ` Roman Kisel
2025-01-17  0:17   ` Jakub Kicinski
2025-01-17 11:10     ` Przemek Kitszel
2025-01-17 19:14       ` Roman Kisel
2025-01-18  3:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).