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 4DB1845E1E for ; Tue, 3 Dec 2024 16:37:07 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5CEE640264; Tue, 3 Dec 2024 16:37:06 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id B599A40264 for ; Tue, 3 Dec 2024 16:37:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733240224; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=4yjgSqG0jAolXgx9fn7/Ojz6UW9Hp0Clqy2lYCvp9io=; b=W7RLhdvTPYhYg0RUhMpD0zDstsSv3mSXWT1wNG8WCAdBDE9qICA/w/qeKgRkZWJYpesbWb 7PwhK71PeaXqC7dhWweWJaBBSETlG8G+lMm1NFuTUHYaoGeCiBYYuJUWgIGpQwpkLQDEnJ 4WiGukahOcm7icg/xIXeWcq72HAJtZs= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-163-nVeM1bnbO8-sApX2u2AmBw-1; Tue, 03 Dec 2024 10:37:02 -0500 X-MC-Unique: nVeM1bnbO8-sApX2u2AmBw-1 X-Mimecast-MFC-AGG-ID: nVeM1bnbO8-sApX2u2AmBw Received: by mail-ed1-f71.google.com with SMTP id 4fb4d7f45d1cf-5d0b5036394so3038107a12.3 for ; Tue, 03 Dec 2024 07:37:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733240221; x=1733845021; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=4yjgSqG0jAolXgx9fn7/Ojz6UW9Hp0Clqy2lYCvp9io=; b=s1WAbzBDpUufVNZfs/IiawhatkMpuNYwp1ZJ23ZTRwnlohEYXcnSoqjq2qtDQ5wuC9 3l3Qr4Tr6klOdH/c+GBx21Eq7w+GY2sEUH4EPkxAlHOcLYutGQkuWVQdJ0w9yobtLlw1 J1rPhr+eFv1mpFGyXChOeLmA11ABOlxo5s3SsZ6+UdC8F3eAURzJOzICg6UbDzgqAyJq VKaKSfpDLWz6KHA7rrszPS5NDzpz6yPX8sWr3EhU9GyGWFdoqwztySlEnJf/wuH4EGxx oRLQ/CvV0335Nzmq10XDSkfcfOAZTfO+glFipJvo94oq3UVNN5e+djLyFhWPF2/JsP86 USiQ== X-Forwarded-Encrypted: i=1; AJvYcCWjh37QFt8L3k9boGQHnUYHWcJIqTViBKlmrc2FDADokcw7E/oqyCx3oYTw3FYW6wj6thBnRbw=@dpdk.org X-Gm-Message-State: AOJu0YwUrUPQ2sLajLW0T6M4yzWn7wROY4k27Lg8a8+tEd5IOIPrMs/q u+DusC32ONijF7jSadwWMGstt+tr0lMRFzf0Il0WcdXVcDd44U3+oYyUPCB+GGT1p+vSUepIFZV /ijwCJ5Au5mBxTN8d7Ih0NbzCBtxDhO5LY4kAU6p7P56j X-Gm-Gg: ASbGncsr7jEQUKS3pjPDZqIZ3BifGH108Q2FcVPit6GOUMYt/DaomkU+Hx0s4D8ig6h i462SfB0a2sJ59tvR8rq2cIRcxVvQA3SEp3OhWhJeZ+GqhsDNSlLxM0FBFAsxiMisM6Ore1nzu6 JVTjCJZVbbKvKzTfTNT8zaSKJGQwej+DVtuXLajCN5YWSIlGc3bLFYuHYw0ZRD9sMHA9EXgJmPE xwwYe2kfoS04RNjH5C31vOv/1lBax/mYAqYz06kVmBurn8rCw== X-Received: by 2002:a05:6402:5216:b0:5d0:e243:8c3d with SMTP id 4fb4d7f45d1cf-5d10cb5662fmr2732933a12.10.1733240221585; Tue, 03 Dec 2024 07:37:01 -0800 (PST) X-Google-Smtp-Source: AGHT+IEOSzIdD6YwInh3O/N8wSquDHbaXgxOgqcXzT3QQsrNOrdLL7CpZ4v2OieL6rz5f6/wEXAQkw== X-Received: by 2002:a05:6402:5216:b0:5d0:e243:8c3d with SMTP id 4fb4d7f45d1cf-5d10cb5662fmr2732837a12.10.1733240219749; Tue, 03 Dec 2024 07:36:59 -0800 (PST) Received: from [192.168.0.36] ([78.17.46.22]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aa599800a99sm626402666b.87.2024.12.03.07.36.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 03 Dec 2024 07:36:59 -0800 (PST) Message-ID: <0fc0cf42-15dc-4a7c-8d65-0d15be85e12f@redhat.com> Date: Tue, 3 Dec 2024 15:36:58 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: patch 'net/ice/base: fix iteration of TLVs in Preserved Fields Area' has been queued to stable release 21.11.9 To: "Richardson, Bruce" , "Pricoco, Fabio" Cc: "Keller, Jacob E" , "Hore, Soumyadeep" , dpdk stable References: <20241127171916.690404-1-ktraynor@redhat.com> <20241127171916.690404-30-ktraynor@redhat.com> From: Kevin Traynor Autocrypt: addr=ktraynor@redhat.com; keydata= xsFNBF2J2awBEADUEPNhgNI+nJNgiTAUcw4YIgVXEoHlsNPyyzG1BEXkWXALy0Y3fNTiw6+r ltWDkF9jzL9kfkecgQ67itGfk1OaBXgSGKuw1PUpxAwX2Bi76LAR6M5OsyGM9TSVVQwARalz hMwRBIZPzPc7or6Pw7jAOJ8SQGJ1Zlp1YJCjrvpe87V1tH/LY8Wnxn/EuoseFmWILAQZAtYS tGjcrAgYn3SPMLR1B0BP5bTBY06vWQjiufH8drenfDnMJAzuBdG1mqjnTqCjULZ3Hunv4xqZ aMnkvL/K5Tj1c12Oe4930EE53LrXIBUltRg5mBudSWHnC7twjH0082HH9f963Z/2UI63SFIT iUvRvAzJYytgy7XnWLQ0+goZBADKYfolOuC0H8VgCaux8u8KFF28Dy+N6TV2KI58jTlyg1Zu l7QwykZpnOkJFiy37Gfbu3YEOzO72cP/S7/A+zvuqkxi63jyEkd+FY99vLt/HN2MUZwRmKDw UPbLkmrs8WU01/POVsqDcfvz7vu2St8hqqTiSIdQGS2zyTKB2/DvPSM3jws3udkIYSuhn+X4 QBiV6lkVZ7DSE6a065gnAauAql+b32Eymy+xnG5jCt1tR+0Cp2VZYCR9OU2gmomUKBDoX/He pSgED01CqYPNjN+TddirwmQX7ep4DtXc8FWvv2g/pq9WZFQk2QARAQABzSNLZXZpbiBUcmF5 bm9yIDxrdHJheW5vckByZWRoYXQuY29tPsLBjgQTAQgAOBYhBAoiOaH51tHF7VYtEI9CINER a+yJBQJdidmsAhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJEI9CINERa+yJoxIP/3VF 2TIgW4ckxhRFCvFu/606bnvCPie88ake4uWVWMAWwcMc4fKEltRWRCpkSVOwgqoMHnyHxK5r kOKzx2CLJMX5TgTMfKzPuaBDHngHLUzl2DStpBzrod0cVg5TShdmmfjY61uxRJKz+DlSkwgJ riADdVF5PPosQXTkKSGf2ombpTGpx/pue9ocjnr3x4SDpRLlnooM6Jf/3Y3Ib4jX6HPEyWuY b+owIIk9y2nRRGPQ6jbqAhsrXd9V+77UL0QuGWloMuKMZFbNg8hbu7X5aFijAbfxj4YUgojS ba7gfGZQan8h32A9KGQWrmsCBc3j2GqEPsX0r05X7cn7WL6IOPgQJ5EiQ7PlazQYVLrvZg9B n0GKK0k6895mLG0ZZ5v/qajOPF52etSmvFD1WUPb4OqaHqGA9ZtMpaKFRt7Y6rpXqKNU1xzW F5KjbTPtTb9WF3An8dciVv+AYUI7totkZYkWvQtgss8lfaX3NKUvXLVxqK0z3dQyr7rF/tYz PneTKypSksjCgaEBLSrsRmM5zKfe7tSNF/fDntfIq/029Jtcw29TcWEP57peNu6TtejewQD9 sTI+oqiXvW2D5l7LNUDYG8eMJp2oT7I0ZSBRvwcbmjH0DtN/bXCCFfCvk8Yic68F3tV1ctix wQARVKDBhT30uCxycRWojCYqTgNJJS71zsFNBF2J2awBEADP57PR2IpSYBeNSrsAjeIcsahE N4SQP2C4s50S8QEWAUhqMRI7WNv5cfeef0nDvcl1IUA6oz5SokbcsbMa+mRgaNF4N5KikWTO LPYxq2YVJoXwJ+tKmNzyOLFUIfFJ4NBJZple5dTfWzD00Dbb19Mri1hy1mWMqNTPGBee1+hw Qcp6n3mmGECvajs8G5A7NyXbwL8ihN7HX9D01ucD62b4G03yKe2g/hvKgcdUVmhCldJlF27I 2fSR9tDxH9pZqRODY4rjbFZEey/vWKXqjE+DQ8AtMSEaDfFe5D+i4Aw6erWQ3Wr+DwZt1/7G dIAElGA/q90T1ENVwJX9y7fsQssawKYYdDqURHCl5JuDXI+VXUypExipUUT5SPycMmbLsx0D iKEqPPDQWKxkIDVKqj2+EhamSuJznZUwBLJKn0h4zrIWiXWUy07lRwtVuhaDXhF3GfW+5W/x wAg7Qg3w00ASsb/XTHBIhMnenKDfS7ihtQA8SacwX8ySdxb+15XPyiplM979qBQ0mhnilulm MIJzEf/JxoYR5huuj4f1PFqqrsP06Dl+YGB7dQZp3IKggS5c3/TAynARRg9N89UsDXNtp7X0 tgIPFF5k6fnHE0J5O64GYHeTqN/1aE6dAEOV9WrGzQAJxU9ipikb8jKAWXzLewRIKGmoPcRZ WdB0NmIjmQARAQABwsF2BBgBCAAgFiEECiI5ofnW0cXtVi0Qj0Ig0RFr7IkFAl2J2awCGwwA CgkQj0Ig0RFr7IkkORAAl/NbX93WK5MEoRw7/DaPTo/Lo6Pj1XMeSqGyACigHK/452UDvlEH NjNJMzYYrNIjMtEmN9VVCfjT38CSca7mpGQVwchc0mC7QSPAETLCS+UacVf/Kwxz5FfkEUUw UT7A+uyVOIgW3d9ldlRzkHA2czonSSgTQU+i2g6DM4ha+BuQb4byAXH6HQHt/Zh1J64z0ohH v6iGsCzCY/sMWF8+LEGSnzMGRCLiiwSF0vJBHbzWK68fANaF4gBV0Z/+6tQRFN7YMhj/INmk qgvHj1ZzHFNtirjMGPRxoZs51YoLQM/aBPxKrnmXThx1ufH+0L6sGmFTugiDt0XSEkC5reH7 a+VhQ1VTFFQrClA8NmDSPzFeuhru4ryaaDHO+uEB16cNHxHrQtlP/2hts2JM5lwkZRWJ5A57 h8eDEIK5be47T85NVHfuTaboNRmgg1HygVejhGUtt69u/0MVRg/roUTa0FyEbNsvz4qAecyW yWzMcVrcGJDQLC9JLKEpoyUF6gdTKaiDL2Vao4+XRIA3Y57b6MO35a3HuzAv7+i5Z0mnDEJO XxXqTOmKYpMIGexzM/PtuA0712sT1abG9tAJ17ao/B7cqMW5IkKkalemFbWfI2unns4Papvo tk9igVqyp6EJDU98z5TJioCVojwK2laDaoIjTJk9YYv3iwCsqPd5feU= In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: i-I9_X-drcgxtvCdNSqb2-bX5sIuQyUByiy5XLKN7oQ_1733240222 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On 03/12/2024 16:10, Richardson, Bruce wrote: > Rather than trying to rework the loop code, I would think it would be better to provide a new local definition of "check_add_overflow", since it's used 3 times in the patch, not just once. The implementation only needs to support u16 values. Something like (again not tested): > > bool > check_add_overflow(u16 a, u16 b, u16 *out) > { > u32 val = (u32)a + (u32)b; > *out = (uint16_t)val; > > return (val > UINT16_MAX); > } > LGTM and better than my version because it will minimise the changes from the main branch commit. Considering its fairly trivial but not tested, do you think it warrants adding to 21.11 LTS ? If so, quickest would be for me to just rebase the existing patch with above code and (with your permission) add your sign-off, or if you prefer you could send the rebased patch. >> -----Original Message----- >> From: Kevin Traynor >> Sent: Tuesday, December 3, 2024 2:13 PM >> To: Pricoco, Fabio ; Richardson, Bruce >> >> Cc: Keller, Jacob E ; Hore, Soumyadeep >> ; dpdk stable >> Subject: Re: patch 'net/ice/base: fix iteration of TLVs in Preserved Fields Area' >> has been queued to stable release 21.11.9 >> >> On 27/11/2024 18:17, Kevin Traynor wrote: >>> Hi, >>> >>> FYI, your patch has been queued to stable release 21.11.9 >>> >> >> Hi Bruce/Fabio, >> >> This patch is causing a build issue for CentOS7 as >> __builtin_add_overflow is not available on GCC 4.8.5. >> >> Though CentOS7 has just gone EoL, I don't want to break the build with >> the last 21.11 LTS release. >> >> I think we could replace the check_add_overflow/builtin with something >> like below (untested), but if the patch isn't important for 21.11 >> branch, then it's probably safer to drop it. >> >> How much is the patch needed for 21.11 branch ? >> >> thanks, >> Kevin. >> >> --- a/drivers/net/ice/base/ice_nvm.c >> +++ b/drivers/net/ice/base/ice_nvm.c >> @@ -427,6 +427,4 @@ enum ice_status ice_read_sr_word(struct ice_hw >> *hw, >> u16 offset, u16 *data) >> } >> >> -#define check_add_overflow __builtin_add_overflow >> - >> /** >> * ice_get_pfa_module_tlv - Reads sub module TLV from NVM PFA >> @@ -458,5 +456,6 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >> } >> >> - if (check_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) { >> + max_tlv = pfa_ptr + pfa_len - 1; >> + if (pfa_ptr > max_tlv) { >> ice_debug(hw, ICE_DBG_INIT, "PFA starts at offset %u. >> PFA length of %u caused 16-bit arithmetic overflow.\n", >> pfa_ptr, pfa_len); >> @@ -475,4 +474,5 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >> while (next_tlv < max_tlv) { >> u16 tlv_sub_module_type; >> + u16 curr_tlv; >> u16 tlv_len; >> >> @@ -499,6 +499,7 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >> } >> >> - if (check_add_overflow(next_tlv, (u16)2, &next_tlv) || >> - check_add_overflow(next_tlv, tlv_len, &next_tlv)) { >> + curr_tlv = next_tlv; >> + next_tlv = next_tlv + tlv_len + 2; >> + if (curr_tlv > next_tlv) { >> ice_debug(hw, ICE_DBG_INIT, "TLV of type %u and >> length 0x%04x caused 16-bit arithmetic overflow. The PFA starts at >> 0x%04x and has length of 0x%04x\n", >> tlv_sub_module_type, tlv_len, >> pfa_ptr, pfa_len); >> >> >>> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. >>> It will be pushed if I get no objections before 12/02/24. So please >>> shout if anyone has objections. >>> >>> Also note that after the patch there's a diff of the upstream commit vs the >>> patch applied to the branch. This will indicate if there was any rebasing >>> needed to apply to the stable branch. If there were code changes for >> rebasing >>> (ie: not only metadata diffs), please double check that the rebase was >>> correctly done. >>> >>> Queued patches are on a temporary branch at: >>> https://github.com/kevintraynor/dpdk-stable >>> >>> This queued commit can be viewed at: >>> https://github.com/kevintraynor/dpdk- >> stable/commit/ebbecb19ccdb0df88227e69b2a83ae2ee79a2d19 >>> >>> Thanks. >>> >>> Kevin >>> >>> --- >>> From ebbecb19ccdb0df88227e69b2a83ae2ee79a2d19 Mon Sep 17 >> 00:00:00 2001 >>> From: Fabio Pricoco >>> Date: Fri, 23 Aug 2024 09:56:42 +0000 >>> Subject: [PATCH] net/ice/base: fix iteration of TLVs in Preserved Fields Area >>> >>> [ upstream commit dcb760bf0f951b404bce33a1dd14906154b58c75 ] >>> >>> The ice_get_pfa_module_tlv() function iterates over the Preserved Fields >>> Area to read data from the Shadow RAM, including the Part Board Assembly >>> data, among others. >>> >>> If the specific TLV being requested is not found in the current NVM, the >>> code will read past the end of the PFA, misinterpreting the last word of >>> the PFA and the word just after the PFA as another TLV. This typically >>> results in one extra iteration before the length check of the while loop >>> is triggered. >>> >>> Correct the logic for determining the maximum PFA offset to include the >>> extra last word. Additionally, make the driver robust against overflows >>> by using check_add_overflow. This ensures that even if the NVM provides >>> bogus data, the driver will not overflow, and will instead log a useful >>> warning message. The check for whether the TLV length exceeds the PFA >>> length is also removed, in favor of relying on the overflow warning >>> instead. >>> >>> Fixes: 5d0b7b5fc491 ("net/ice/base: add read PBA module function") >>> >>> Signed-off-by: Fabio Pricoco >>> Signed-off-by: Jacob Keller >>> Signed-off-by: Soumyadeep Hore >>> Acked-by: Bruce Richardson >>> --- >>> drivers/net/ice/base/ice_nvm.c | 36 ++++++++++++++++++++++------------ >>> 1 file changed, 24 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/net/ice/base/ice_nvm.c b/drivers/net/ice/base/ice_nvm.c >>> index 48e0d418e2..c5a3eddebf 100644 >>> --- a/drivers/net/ice/base/ice_nvm.c >>> +++ b/drivers/net/ice/base/ice_nvm.c >>> @@ -427,4 +427,6 @@ enum ice_status ice_read_sr_word(struct ice_hw >> *hw, u16 offset, u16 *data) >>> } >>> >>> +#define check_add_overflow __builtin_add_overflow >>> + >>> /** >>> * ice_get_pfa_module_tlv - Reads sub module TLV from NVM PFA >>> @@ -443,6 +445,5 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >>> { >>> enum ice_status status; >>> - u16 pfa_len, pfa_ptr; >>> - u32 next_tlv; >>> + u16 pfa_len, pfa_ptr, next_tlv, max_tlv; >>> >>> status = ice_read_sr_word(hw, ICE_SR_PFA_PTR, &pfa_ptr); >>> @@ -456,9 +457,21 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >>> return status; >>> } >>> - /* Starting with first TLV after PFA length, iterate through the list >>> + >>> + if (check_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) { >>> + ice_debug(hw, ICE_DBG_INIT, "PFA starts at offset %u. PFA >> length of %u caused 16-bit arithmetic overflow.\n", >>> + pfa_ptr, pfa_len); >>> + return ICE_ERR_INVAL_SIZE; >>> + } >>> + >>> + /* The Preserved Fields Area contains a sequence of TLVs which define >>> + * its contents. The PFA length includes all of the TLVs, plus its >>> + * initial length word itself, *and* one final word at the end of all >>> + * of the TLVs. >>> + * >>> + * Starting with first TLV after PFA length, iterate through the list >>> * of TLVs to find the requested one. >>> */ >>> next_tlv = pfa_ptr + 1; >>> - while (next_tlv < ((u32)pfa_ptr + pfa_len)) { >>> + while (next_tlv < max_tlv) { >>> u16 tlv_sub_module_type; >>> u16 tlv_len; >>> @@ -477,8 +490,4 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >>> break; >>> } >>> - if (tlv_len > pfa_len) { >>> - ice_debug(hw, ICE_DBG_INIT, "Invalid TLV length.\n"); >>> - return ICE_ERR_INVAL_SIZE; >>> - } >>> if (tlv_sub_module_type == module_type) { >>> if (tlv_len) { >>> @@ -489,8 +498,11 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 >> *module_tlv, u16 *module_tlv_len, >>> return ICE_ERR_INVAL_SIZE; >>> } >>> - /* Check next TLV, i.e. current TLV pointer + length + 2 words >>> - * (for current TLV's type and length) >>> - */ >>> - next_tlv = next_tlv + tlv_len + 2; >>> + >>> + if (check_add_overflow(next_tlv, (u16)2, &next_tlv) || >>> + check_add_overflow(next_tlv, tlv_len, &next_tlv)) { >>> + ice_debug(hw, ICE_DBG_INIT, "TLV of type %u and >> length 0x%04x caused 16-bit arithmetic overflow. The PFA starts at 0x%04x >> and has length of 0x%04x\n", >>> + tlv_sub_module_type, tlv_len, >> pfa_ptr, pfa_len); >>> + return ICE_ERR_INVAL_SIZE; >>> + } >>> } >>> /* Module does not exist */ >