Skip to contentSkip to conversation

hv_netvsc: Replace one-element array with flexible array member

PATCH Applied Thorsten Blum 5 participants 6 messages Jan 16, 2025 – Jan 18, 2025

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 hv_netvsc: Replace one-element array with flexible array member

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.

---
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/[email protected]/

---
 drivers/net/hyperv/hyperv_net.h | 2 +-
 drivers/net/hyperv/netvsc.c     | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/net/hyperv/hyperv_net.h +1 -1
index e690b95b1bbb..234db693cefa 100644
@@ -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;
 
 /*
drivers/net/hyperv/netvsc.c +2 -1
index 9afb08dbc350..d6f5b9ea3109 100644
@@ -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;
Signature · 1 line
2.48.0
#1 lore raw
Quoted 4 lines · Thorsten Blum

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!

Quoted 2 lines

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.

Quoted 40 lines

---
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/[email protected]/

---
  drivers/net/hyperv/hyperv_net.h | 2 +-
  drivers/net/hyperv/netvsc.c     | 3 ++-
  2 files changed, 3 insertions(+), 2 deletions(-)
drivers/net/hyperv/hyperv_net.h +1 -1
index e690b95b1bbb..234db693cefa 100644
@@ -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;
 
  /*
drivers/net/hyperv/netvsc.c +2 -1
index 9afb08dbc350..d6f5b9ea3109 100644
@@ -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;
Signature · 2 lines
Thank you,
Roman
maintainer
#2 lore raw
Quoted 9 lines · On Thu, 16 Jan 2025 13:39:52 -0800 Roman Kisel wrote:
Quoted 4 lines · Thorsten Blum

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!

Quoted 1 line
Quoted 7 lines
Quoted 1 line

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 ..

#3 lore raw
Quoted 6 lines · Jakub Kicinski
Quoted 5 lines · On Thu, 16 Jan 2025 13:39:52 -0800 Roman Kisel wrote:
Quoted 4 lines · Thorsten Blum

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".

Quoted 6 lines
Quoted 6 lines

Thanks!

Quoted 2 lines

Compile-tested only.

Quoted 7 lines
Quoted 5 lines

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.

Quoted 3 lines

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

Tested-by Reviewed-by
#4 lore raw
Quoted 12 lines · Przemek Kitszel
Quoted 6 lines · Jakub Kicinski
Quoted 5 lines · On Thu, 16 Jan 2025 13:39:52 -0800 Roman Kisel wrote:
Quoted 4 lines · Thorsten Blum

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.

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines

Thanks!

Quoted 2 lines

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.

Quoted 4 lines
Quoted 1 line

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!

Quoted 5 lines
Quoted 4 lines

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

Signature · 2 lines
Thank you,
Roman
#5 lore raw
In reply to Thorsten Blum
This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 8 lines · 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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html