linux-kernel - RE: [PATCH v2] 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: <BYAPR21MB1688FA593FF45C7BCCD74BB9D715A@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Wed, 16 Aug 2023 04:52:58 +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 v2] hv: hyperv.h: Replace one-element array with
 flexible-array member

From: Saurabh Sengar <ssengar@...ux.microsoft.com> Sent: Tuesday, August 15, 2023 9:35 PM
> 
> 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 */
> };
> 
> The validation code in the netvsc driver is affected by changing the
> struct size, but the effects have been examined and have been determined
> to be appropriate.
> 
> Signed-off-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> ---
> [V2]
>  - Added more info in commit message regarding netvsc validation code
>    affected by change of this struct.
> 
>  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 3ac3974b3c78..5c66640ea8db 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;
> 
>  struct vmgpadl_packet_header {
> --
> 2.34.1

Reviewed-by: Michael Kelley <mikelley@...rosoft.com>