Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
From:   Yuran Pereira <yuran.pereira@hotmail.com>
To:     gregkh@linuxfoundation.org, yuran.pereira@hotmail.com
Cc:     bcm-kernel-feedback-list@broadcom.com, davem@davemloft.net,
        edumazet@google.com, florian.fainelli@broadcom.com,
        justin.chen@broadcom.com, kuba@kernel.org,
        linux-kernel-mentees@lists.linuxfoundation.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        pabeni@redhat.com
Subject: Re: [PATCH] Prevent out-of-bounds read/write in bcmasp_netfilt_rd and bcmasp_netfilt_wr
Date:   Fri,  3 Nov 2023 19:49:00 +0530
Message-ID: <DB3PR10MB683569338168B85CB009A5E8E8A5A@DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM>
In-Reply-To: <DB3PR10MB68352DF6CB458CCF97C416ECE8A5A@DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM>
References: <DB3PR10MB68352DF6CB458CCF97C416ECE8A5A@DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: DB3PR10MB6835:EE_|AM0PR10MB3331:EE_
X-MS-Office365-Filtering-Correlation-Id: a77bc5eb-178e-4323-5c20-08dbdc77de16
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: WZ6dRuSPIOjOsceLX/6WORqs5W1BwHEjT/KrvjFoNd+zCCx6/A5G9vRAx6PI0BFLHboAdyygGQZE+XBKSIbhr0I3EFEFFrt33jdICh6gfaeUjpDs5Zw1lJzju9+uQQAipD0mrj6XSdQ9FpVxYx41FFB0hPhGanOmXAZq+JEheNKCUjdcrJoGYJUYW2NWiES9AbxXNdbrJBj373wxk8ppzRUTeWwnWtkkriQHgR04B3cswHRMuLPp13/EZB9TxvFLaf0RyjWPNtwahVd1ROsigTaI5Cf7Rinjh05fq+UxNZS4qLIr1RCgt26CvscbSM1sEcn+EO3ALySGOIWHp4TuG1l0zab/HlqkuGzd/7fB7y4fpSa/4wmvcNIfm4DfQSB+h2+P8UM68W/8owU4rUe0lnf0R2wli26m6N8dx17W9TCTXuxOEjPNMkOS/WSalPuE6Qn02AbnPECEmLyxObqWe97gPMFlccid2gAFnQfcFbzPWnFsQAoidQwlDyzXjTpIrDurXA+cswOhJ3jLNz4GJK5hMoWwHPNofmNampWIrq5TzTrmTu1mGKvisO30x0lQIOzIGGZ0sjie3UygHFRrM8ytYEJ8stUKFRBhadC1b/OYgpMVuSjjwbeXm9n21A4I
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Cse4UXeTk3Cldc4FlpAzcTxFjuoQM5Cr+0UKo6ohVwn6UHjVjbobRyTBHc70?=
 =?us-ascii?Q?lLdtgYcNuI05OO/UzdWMQt0EJ2RSNHegIoxaUtAXYp2v1YYit/GuBrJp9sdJ?=
 =?us-ascii?Q?ccbCXl/p69VXvvG+rvjahCDpHBpoLHUwKPCmYJ6d8zfiR5ca6B8L0+ineM+w?=
 =?us-ascii?Q?grkhxW7SO/yfOwVJJKrHm5KAyMsNdGu42r8vAbcnUtxjkII4G4Fkobp5y/Ho?=
 =?us-ascii?Q?Ph+xOZBjSNYx1t/+2bYEWve7OW+u8K9X5UcRy2JeRexCeU1WlkfhqYm5jD2S?=
 =?us-ascii?Q?XuOxCyxGhUXByo61sW9FZ10PtertbhSyV8c+59HZuqtwsLmu5GZAOSnq8q+2?=
 =?us-ascii?Q?SgaaZGhNjFAylqQnzslOQgKvYI5eoQurYpAyLEESvhuzwp0ha0vy8oI3Dg0E?=
 =?us-ascii?Q?8lvXlkYxX+Vuj8OAraM8hqy2CYBQWAl+sZBtWo4VxrBfNWuEf//s9veTUk1l?=
 =?us-ascii?Q?BQv0mx1wOlYT/6F1uQapSZy50Pic7RXl04rasw6lvotttut4QzR8TuSIbZ+Z?=
 =?us-ascii?Q?U7vhGZb1973JxB1f0Eshp6quIzwfqJRZXOQ+OF0vF3Zaw3YPpmPOgleBzTBY?=
 =?us-ascii?Q?z+rsIagRMjo/ZHtkM10a/cXQOElhmgDBJ6vwcHI/JXH7VCFhgW07/KH2Zkgb?=
 =?us-ascii?Q?MY3SgGxQJ81pdYmfdsB68ZJxU4DHAqsV1U8LoLNj0SaFWm1MvBg0l1m7Rk9R?=
 =?us-ascii?Q?kSEJWabRAHACI6Mg1GRqAk9pTqO478y4Ug0mA0dYePlhw+oDhpHV06fUJYKJ?=
 =?us-ascii?Q?ZKIvAe4SI4qEhHbNI/4CVLlNBUk7an+s0mkU2V5VFjHfmiteCjkCLC2n/oCX?=
 =?us-ascii?Q?rOb2fW7mZtT5lC7Y26RQl4UiQqm467YmH8jqPbzxaqd40MyIVMRdzSBLCHAa?=
 =?us-ascii?Q?aGWp0Gobn8ROiPSGoW9QBfaAY5fxVagdPUBCA+NfFvn5YIHgx9TDINeyNsqQ?=
 =?us-ascii?Q?uaet6Hyl7YtDoJ2haea5AnBJ5vM9Gheg1XD9xOL3qVE2EKXbW7aFhKzMIMIr?=
 =?us-ascii?Q?1zmxSfPGxcRsMqP4ZHGI/3SB9GRtjwZDr6b4kSF5GYsXPdfJbdBNw7x4Bw0J?=
 =?us-ascii?Q?8/TBbgn6Og1O9YyreRF+BHhmUNiXHZgz3+s7z/dDjGddaR11xlRog2q70lu/?=
 =?us-ascii?Q?KA4FySu7ZcB6LftMSAU2ieBx+TiZPbUO87GtTz9kCthN2/hZVpeHzZ4jBVlT?=
 =?us-ascii?Q?Bam3YuIjPFGPJYlnlMMod9lzmaZHAyN9ksimERtsXZBCjW5Wmx86ucn2H3o?=
 =?us-ascii?Q?=3D?=
X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-6b909.templateTenant
X-MS-Exchange-CrossTenant-Network-Message-Id: a77bc5eb-178e-4323-5c20-08dbdc77de16
X-MS-Exchange-CrossTenant-AuthSource: DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2023 14:19:19.4579
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR10MB3331
X-Spam-Status: No, score=0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,FORGED_HOTMAIL_RCVD2,FREEMAIL_FORGED_FROMDOMAIN,
	FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Fri, 03 Nov 2023 07:19:31 -0700 (PDT)


On a second thought, it might not be a good idea to return
an error without modifying the caller, since the caller of
this function currently uses this return value without checking
if it's an error.
I guess that explains why the first check returns 0.

```
static int bcmasp_netfilt_wr_m_wake(struct bcmasp_priv *priv,
...
{
		...
        if (first_byte && (!IS_ALIGNED(offset, 4) || size < 3)) {
            match_val = bcmasp_netfilt_rd(priv, nfilt,
                              ASP_NETFILT_MATCH,
                              ALIGN_DOWN(offset, 4));
            mask_val = bcmasp_netfilt_rd(priv, nfilt,
                             ASP_NETFILT_MASK,
                             ALIGN_DOWN(offset, 4));
        }

        shift = (3 - (offset % 4)) * 8;
        match_val &= ~GENMASK(shift + 7, shift);
        mask_val &= ~GENMASK(shift + 7, shift);
        match_val |= (u32)(*((u8 *)match) << shift);
        mask_val |= (u32)(*((u8 *)mask) << shift);

```