From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 582B54334B; Fri, 17 Nov 2023 03:46:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 07A1F40271; Fri, 17 Nov 2023 03:45:59 +0100 (CET) Received: from smtpbgjp3.qq.com (smtpbgjp3.qq.com [54.92.39.34]) by mails.dpdk.org (Postfix) with ESMTP id 757E24014F; Fri, 17 Nov 2023 03:45:56 +0100 (CET) X-QQ-mid: Yeas4t1700189144t149t62650 Received: from 3DB253DBDE8942B29385B9DFB0B7E889 (jiawenwu@trustnetic.com [183.128.129.197]) X-QQ-SSF: 00400000000000F0FSF000000000000 From: =?utf-8?b?Smlhd2VuIFd1?= X-BIZMAIL-ID: 9740102383196155269 To: , "'Jian Wang'" , "'Ferruh Yigit'" Cc: , , "'Luca Boccassi'" References: <20231116140718.4026676-1-ferruh.yigit@amd.com> In-Reply-To: <20231116140718.4026676-1-ferruh.yigit@amd.com> Subject: RE: [PATCH] net/txgbe: fix out of bound access Date: Fri, 17 Nov 2023 10:45:43 +0800 Message-ID: <0c7201da1900$28c7d450$7a577cf0$@trustnetic.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQJWdiSBtY/LVQDQQGP2R0+NZhA006+FEpnw Content-Language: zh-cn X-QQ-SENDSIZE: 520 Feedback-ID: Yeas:trustnetic.com:qybglogicsvrgz:qybglogicsvrgz5a-1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thursday, November 16, 2023 10:07 PM, Ferruh.Yigit@amd.com wrote: > Reported by SuSe CI [1] by GCC [2], possibly false positive. Error: > > In function 'txgbe_host_interface_command', > inlined from 'txgbe_host_interface_command' > at ../drivers/net/txgbe/base/txgbe_mng.c:104:1, > inlined from 'txgbe_hic_reset' > at ../drivers/net/txgbe/base/txgbe_mng.c:345:9: > ../drivers/net/txgbe/base/txgbe_mng.c:145:36: > error: array subscript 2 is outside array bounds ofr > 'struct txgbe_hic_reset[1]' [-Werror=array-bounds=] > 145 | buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi); > ../drivers/net/txgbe/base/txgbe_mng.c: In function 'txgbe_hic_reset': > ../drivers/net/txgbe/base/txgbe_mng.c:331:32: > note: at offset 8 into object 'reset_cmd' of size 8 > 331 | struct txgbe_hic_reset reset_cmd; > | ^~~~~~~~~ > > Access to buffer done based on command code, the case complained by > FW_RESET_CMD has short buffer but this code path only taken with command > 0x30, so this shouldn't be a problem. > > Adding a size check before accessing to the buffer, as this is control > plane code, additional check shouldn't hurt. > > [1] > https://build.opensuse.org/public/build/home:bluca:dpdk/openSUSE_Factory_ARM/armv7l/dpdk-20.11/_log > > [2] > gcc 13.2.1 "cc (SUSE Linux) 13.2.1 20230912 > > Fixes: 35c90ecccfd4 ("net/txgbe: add EEPROM functions") > Cc: stable@dpdk.org > > Reported-by: Luca Boccassi > Signed-off-by: Ferruh Yigit > --- > Cc: jiawenwu@trustnetic.com > Cc: jianwang@trustnetic.com > > @Luca, I am not sure if this additional check will satisfy the compiler, > can you please verify the patch? > > @Jiawen, there is a specific handling for command 0x30, from comment it > looks like it is Read Flash command, but it looks like this command is > not used by the driver, if this is correct can we remove the check > completely? Removing can be simpler way to fix the compiler error. Thanks Ferruh. This command has been removed because flash can be read directly by the driver. The check can be simply removed. > --- > drivers/net/txgbe/base/txgbe_mng.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/txgbe/base/txgbe_mng.c b/drivers/net/txgbe/base/txgbe_mng.c > index df7145094f84..9797b1b8b5da 100644 > --- a/drivers/net/txgbe/base/txgbe_mng.c > +++ b/drivers/net/txgbe/base/txgbe_mng.c > @@ -147,6 +147,10 @@ txgbe_host_interface_command(struct txgbe_hw *hw, u32 *buffer, > * two byes instead of one byte > */ > if (resp->cmd == 0x30) { > + if (length < ((dword_len + 2) << 2)) { > + err = TXGBE_ERR_HOST_INTERFACE_COMMAND; > + goto rel_out; > + } > for (; bi < dword_len + 2; bi++) > buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi); > > -- > 2.34.1 >