linux-kernel - RE: [PATCH] hv: hyperv.h: Replace one-element array with flexible-array member

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR21MB16888C4F0D90CB84716B49EFD70CA@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Mon, 7 Aug 2023 16:56:28 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Saurabh Sengar <ssengar@...ux.microsoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Saurabh Singh Sengar <ssengar@...rosoft.com>
Subject: RE: [PATCH] hv: hyperv.h: Replace one-element array with
 flexible-array member

From: Saurabh Sengar <ssengar@...ux.microsoft.com> Sent: Monday, August 7, 2023 2:51 AM
> 
> One-element and zero-length arrays are deprecated. Replace one-element
> array in struct vmtransfer_page_packet_header with flexible-array
> member. This change fixes below warning:
> 
> [    2.593788]
> ================================================================================
> [    2.593908] UBSAN: array-index-out-of-bounds in drivers/net/hyperv/netvsc.c:1445:41
> [    2.593989] index 1 is out of range for type 'vmtransfer_page_range [1]'
> [    2.594049] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 6.5.0-rc4-next-20230803+ #1
> [    2.594114] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 04/20/2023
> [    2.594121] Call Trace:
> [    2.594126]  <IRQ>
> [    2.594133]  dump_stack_lvl+0x4c/0x70
> [    2.594154]  dump_stack+0x14/0x20
> [    2.594162]  __ubsan_handle_out_of_bounds+0xa6/0xf0
> [    2.594224]  netvsc_poll+0xc01/0xc90 [hv_netvsc]
> [    2.594258]  __napi_poll+0x30/0x1e0
> [    2.594320]  net_rx_action+0x194/0x2f0
> [    2.594333]  __do_softirq+0xde/0x31e
> [    2.594345]  __irq_exit_rcu+0x6b/0x90
> [    2.594357]  irq_exit_rcu+0x12/0x20
> [    2.594366]  sysvec_hyperv_callback+0x84/0x90
> [    2.594376]  </IRQ>
> [    2.594379]  <TASK>
> [    2.594383]  asm_sysvec_hyperv_callback+0x1f/0x30
> [    2.594394] RIP: 0010:pv_native_safe_halt+0xf/0x20
> [    2.594452] Code: 0b 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90
> 90 90 90 90 90 90 f3 0f 1e fa 66 90 0f 00 2d 05 35 3f 00 fb f4 <c3> cc cc cc cc 66 2e 0f
> 1f 84 00 00 00 00 00 66 90 90 90 90 90 90
> [    2.594459] RSP: 0018:ffffb841c00d3e88 EFLAGS: 00000256
> [    2.594469] RAX: ffff9d18c326f4a0 RBX: ffff9d18c031df40 RCX: 4000000000000000
> [    2.594475] RDX: 0000000000000001 RSI: 0000000000000082 RDI: 00000000000268dc
> [    2.594481] RBP: ffffb841c00d3e90 R08: 00000066a171109b R09: 00000000d33d2600
> [    2.594486] R10: 000000009a41bf00 R11: 0000000000000000 R12: 0000000000000001
> [    2.594491] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [    2.594501]  ? ct_kernel_exit.constprop.0+0x7d/0x90
> [    2.594513]  ? default_idle+0xd/0x20
> [    2.594523]  arch_cpu_idle+0xd/0x20
> [    2.594532]  default_idle_call+0x30/0xe0
> [    2.594542]  do_idle+0x200/0x240
> [    2.594553]  ? complete+0x71/0x80
> [    2.594613]  cpu_startup_entry+0x24/0x30
> [    2.594624]  start_secondary+0x12d/0x160
> [    2.594634]  secondary_startup_64_no_verify+0x17e/0x18b
> [    2.594649]  </TASK>
> [    2.594656]
> ================================================================================
> 
> With this change the structure size is reduced by 8 bytes, below is the
> pahole output.
> 
> struct vmtransfer_page_packet_header {
> 	struct vmpacket_descriptor d;                    /*     0    16 */
> 	u16                        xfer_pageset_id;      /*    16     2 */
> 	u8                         sender_owns_set;      /*    18     1 */
> 	u8                         reserved;             /*    19     1 */
> 	u32                        range_cnt;            /*    20     4 */
> 	struct vmtransfer_page_range ranges[];           /*    24     0 */
> 
> 	/* size: 24, cachelines: 1, members: 6 */
> 	/* last cacheline: 24 bytes */
> };
> 
> Signed-off-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> ---
>  include/linux/hyperv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index bfbc37ce223b..c529f407bfb8 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -348,7 +348,7 @@ struct vmtransfer_page_packet_header {
>  	u8  sender_owns_set;
>  	u8 reserved;
>  	u32 range_cnt;
> -	struct vmtransfer_page_range ranges[1];
> +	struct vmtransfer_page_range ranges[];
>  } __packed;
> 

As you noted, switching to a flexible array member changes the
size of struct vmtransfer_page_packet_header.   In netvsc_receive()
the size of this structure is used in validation of the VMbus packet
received from Hyper-V.  Changing the size of the structure changes
the validation.   The validation code probably needs to be adjusted
to account for the new structure size (or the original validation code
was wrong).

There might be other places in the code that are affected by a change
in the size of this structure.  I haven't fully investigated.

Michael