Message ID | 1691401853-26974-1-git-send-email-ssengar@linux.microsoft.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp1360400vqr; Mon, 7 Aug 2023 03:49:44 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG8UWOloxAaiqIHW0A32SOep+XfxQ3CN6nwQxifLnIHpLzIrD2cbcY46AB2gs9Bx8mZpKuw X-Received: by 2002:a05:6808:2010:b0:3a7:459d:61bf with SMTP id q16-20020a056808201000b003a7459d61bfmr9902362oiw.0.1691405384635; Mon, 07 Aug 2023 03:49:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691405384; cv=none; d=google.com; s=arc-20160816; b=OCwT5U1dBX6ebts0jrn6r8f+rApde23gGNhRbaGPpyvQOw/n9wTM2ZUV5LGQddpC0n qH3vWw2WXStljA096RGTfWwJvp39J5VNehxGiipBf7bAAYEtu1nvFyP8eSbNIw56GAje EsbBKNjVhxq+NUzwKBv2A3thte4x48uG1wAauREHudiRWEqR8VKs+Vj8BdyuhWVsrvPn hpEYbm0Akmwm/oa6TyNMCv6IdOk6CrjY2+fkAnIKE3AUbT/zeQA5UkcSvnZnrBN9d5s3 Ed7q7+hwKglVMJ2t1d0mIlJmkHpX5tjP+9y0Z+j1cwbvyKcqNgdATd4IUw8JeEY6BjYr yvzw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature:dkim-filter; bh=I8JPvW2x4wvhQ88X64Egmr5ves+Y3CqToeE6x4H3cos=; fh=4XfK2L1mvaISF8Rh6BQugJF5Fj9lfR/wOyBz+pwMLb8=; b=Y6nhnhBQVY6E0aTHOyoMwGJqamatTchUWaxs3udPezzOkzEh9FGeQZAvYE5GwizYDi GgFGwsVopTRRvTDIL0V9Mp+7NqXqb/KiGeSSvJSCUdslMfIpQjeyZXE4LBWbLJyr+SQm FbImM1LECBYbwO7s8xI47HtlCVkh+DCH9qvFixZstZEGFk1UR3fnbkcVqVICDBSp1idS CmHDzXMEvORUAFrJaFc5bjFukvmAtjXMQd7yAp1qQcp0IhOQS9tKXW56wfwWd/pQx2ZE T2tnLvOrkkHkLRctlqNZU3Tcbap3kMFAC1k2U3rczNEwJzv3Ps2wiKiJ46miDhHXCZ8s /tbg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=l+M2Sgvx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a21-20020a056a001d1500b00686f001cb5esi5474024pfx.319.2023.08.07.03.49.31; Mon, 07 Aug 2023 03:49:44 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=l+M2Sgvx; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229587AbjHGJwH (ORCPT <rfc822;aaronkmseo@gmail.com> + 99 others); Mon, 7 Aug 2023 05:52:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39002 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229998AbjHGJwE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 7 Aug 2023 05:52:04 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id C316D1BCD; Mon, 7 Aug 2023 02:51:44 -0700 (PDT) Received: from linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net (linux.microsoft.com [13.77.154.182]) by linux.microsoft.com (Postfix) with ESMTPSA id 4C043208694F; Mon, 7 Aug 2023 02:51:02 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4C043208694F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1691401862; bh=I8JPvW2x4wvhQ88X64Egmr5ves+Y3CqToeE6x4H3cos=; h=From:To:Cc:Subject:Date:From; b=l+M2Sgvx6w0bgLv9XXrQoQ/gVH46rV69/VdKwlhKbdqKgMZ2A5+EXQ9B7lutxuq76 U9QQGAGv0/Mops1yLwpjZ2Pc4I69vqAwCV80g2KHbLkBh5ZLTCz0mx2WBAa/ONFa5D LYFVdF5MNiL8PMglP0jIoH2zNyyQIxz/KJ4WOIW0= From: Saurabh Sengar <ssengar@linux.microsoft.com> To: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, ssengar@microsoft.com Subject: [PATCH] hv: hyperv.h: Replace one-element array with flexible-array member Date: Mon, 7 Aug 2023 02:50:53 -0700 Message-Id: <1691401853-26974-1-git-send-email-ssengar@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 X-Spam-Status: No, score=-19.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS,USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773567092582984671 X-GMAIL-MSGID: 1773567092582984671 |
Series |
hv: hyperv.h: Replace one-element array with flexible-array member
|
|
Commit Message
Saurabh Singh Sengar
Aug. 7, 2023, 9:50 a.m. UTC
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@linux.microsoft.com>
---
include/linux/hyperv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
From: Saurabh Sengar <ssengar@linux.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@linux.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
> -----Original Message----- > From: Michael Kelley (LINUX) <mikelley@microsoft.com> > Sent: Monday, August 7, 2023 10:26 PM > To: Saurabh Sengar <ssengar@linux.microsoft.com>; KY Srinivasan > <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; > wei.liu@kernel.org; Dexuan Cui <decui@microsoft.com> > Cc: linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; Saurabh > Singh Sengar <ssengar@microsoft.com> > Subject: RE: [PATCH] hv: hyperv.h: Replace one-element array with flexible- > array member > > From: Saurabh Sengar <ssengar@linux.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@linux.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. Thanks for your comment, I wanted to have this discussion. Before sending this patch, I was contemplating whether or not to make this change. Through my analysis, I arrived at the conclusion that the initial validation code wasn't entirely accurate. And with the proposed changes it gets more accurate. IMHO it is more accurate to exclude the size of 'ranges' in the header. With my limited understanding of this driver, the current "header size validation" is only to make sure that header is correct. So, that we fetch the range_cnt and xfer_pageset_id correctly from it. For this to be done I don't find any reason to include the size of ranges in this check. With inclusion of ranges we are checking the first 'struct vmtransfer_page_range' size as well which is not required for fetching above two values. Once we have the count of ranges we will anyway check the sanity of ranges with NETVSC_XFER_HEADER_SIZE. This will check "count * (struct vmtransfer_page_range)" Which is present few lines after. For a ranges count = 1, I don't see there is any difference between both the checks as of today. Please let me know you opinion if you don't find my explanation reasonable. I don't see any other place this structure's size change will affect. - Saurabh > > Michael
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; struct vmgpadl_packet_header {