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 BCEA843346; Thu, 16 Nov 2023 16:16:31 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4947E402B0; Thu, 16 Nov 2023 16:16:31 +0100 (CET) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by mails.dpdk.org (Postfix) with ESMTP id 6D65240150; Thu, 16 Nov 2023 16:16:30 +0100 (CET) Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-408382da7f0so7681525e9.0; Thu, 16 Nov 2023 07:16:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700147790; x=1700752590; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=mpvd69E3Bg29dNhg87Oj8CVIzFEKqFMKY6dOTkFvu1w=; b=jbE7lxYgrTypSQRyj4rTNHtKFYU4ZR1VqgGmlecw+axUsNVlNLvlutyux24XHsIQuo MOHRH/OET7Tm06a4toU/wOUc49PbiuJlnfWyp2AG91ah9flCuQjCzJ6G6/HrbG+9yskP NwXjyy2QvlEu5rb56/LC+0j020Qc3jwYT/oKdgRlWuYOS2WdxLmOa1iHM+HSyPn+CcqZ 8bOI4KI7DqQM8tJ7BzRcV4HuyaIkMNG64NUF74gPi4dmICSiqjK9b0G95BSKarlssfQN UWDUzWZ0cgl0mmJmCNqR/Wnn+Kk1cKqM1FIJJfouqdkS3umCfSLZ2yZiR+6RWBs11hEc W1Aw== X-Gm-Message-State: AOJu0YzD1HtreWCDqi1GG8VFcC6i9Kh5/lb906E3WTvNXiOVICl8f8Er 2KGrbnsdKSdUxqJw4C7Y1R8= X-Google-Smtp-Source: AGHT+IFPInkpz8oRgWQUy8GGphrGzOjw8NPhlKdqshZCpUMSOjovCPF1evEDqnNK7t696mvCrrz7mg== X-Received: by 2002:a05:600c:4446:b0:408:59d4:f3c4 with SMTP id v6-20020a05600c444600b0040859d4f3c4mr2477528wmn.10.1700147789550; Thu, 16 Nov 2023 07:16:29 -0800 (PST) Received: from localhost ([2a01:4b00:d307:1000:f1d3:eb5e:11f4:a7d9]) by smtp.gmail.com with ESMTPSA id x18-20020a05600c421200b00406443c8b4fsm3853687wmh.19.2023.11.16.07.16.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Nov 2023 07:16:28 -0800 (PST) Message-ID: Subject: Re: [PATCH] net/txgbe: fix out of bound access From: Luca Boccassi To: Ferruh Yigit Cc: dev@dpdk.org, stable@dpdk.org Date: Thu, 16 Nov 2023 15:16:27 +0000 In-Reply-To: <20231116140718.4026676-1-ferruh.yigit@amd.com> References: <20231116140718.4026676-1-ferruh.yigit@amd.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4-2 MIME-Version: 1.0 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 Thu, 2023-11-16 at 14:07 +0000, Ferruh Yigit wrote: > Reported by SuSe CI [1] by GCC [2], possibly false positive. Error: >=20 > 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=3Darray-bounds=3D] > 145 | buffer[bi] =3D 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; > | ^~~~~~~~~ >=20 > 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. >=20 > Adding a size check before accessing to the buffer, as this is control > plane code, additional check shouldn't hurt. >=20 > [1] > https://build.opensuse.org/public/build/home:bluca:dpdk/openSUSE_Factory_= ARM/armv7l/dpdk-20.11/_log >=20 > [2] > gcc 13.2.1 "cc (SUSE Linux) 13.2.1 20230912 >=20 > Fixes: 35c90ecccfd4 ("net/txgbe: add EEPROM functions") > Cc: stable@dpdk.org >=20 > Reported-by: Luca Boccassi > Signed-off-by: Ferruh Yigit > --- > Cc: jiawenwu@trustnetic.com > Cc: jianwang@trustnetic.com >=20 > @Luca, I am not sure if this additional check will satisfy the compiler, > can you please verify the patch? >=20 > @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. > --- > drivers/net/txgbe/base/txgbe_mng.c | 4 ++++ > 1 file changed, 4 insertions(+) >=20 > 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, u3= 2 *buffer, > * two byes instead of one byte > */ > if (resp->cmd =3D=3D 0x30) { > + if (length < ((dword_len + 2) << 2)) { > + err =3D TXGBE_ERR_HOST_INTERFACE_COMMAND; > + goto rel_out; > + } > for (; bi < dword_len + 2; bi++) > buffer[bi] =3D rd32a(hw, TXGBE_MNGMBX, bi); >=20 Thanks, this fixes the build: https://build.opensuse.org/package/live_build_log/home:bluca:dpdk/dpdk-20.1= 1/openSUSE_Factory_ARM/armv7l Tested-by: Luca Boccassi