From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 60014C2BA19 for ; Tue, 14 Apr 2020 12:36:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 369EF2076B for ; Tue, 14 Apr 2020 12:36:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="Dxy2k4OH" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2502047AbgDNMf6 (ORCPT ); Tue, 14 Apr 2020 08:35:58 -0400 Received: from aserp2120.oracle.com ([141.146.126.78]:60226 "EHLO aserp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2502029AbgDNMfr (ORCPT ); Tue, 14 Apr 2020 08:35:47 -0400 Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 03ECI54q101846; Tue, 14 Apr 2020 12:35:38 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2020-01-29; bh=y60fSCG2nXRrowJmU8VdzYgVbRBn5fWc0JMdkv+EuI8=; b=Dxy2k4OH6mHEFSQyrfvHAoREOtd2VgID+YF/O9/WcuKsGrDCDD+OyMHviTlzFbdZSWyL +eC5JXZL+R2akjNx7bJ73f4wWH3vqUueLewZdsf6DKh2ZRmi/5ki45AjvYVnPPdcfH4K 45aJcolFwvCupJX/QPnYtf8fiyr5J/QCg+tfy/8kCGCSzlLbskY4eFK6pqng4y2J3Vj0 ECfFgNN87QUlB6Cvq+RHMJO8eJqC6jJO44piUF8ibLNlF1tc2J6cCOCZ8w2Go7on7YJv /wdKmbY2lVSJ1q1FOkXtBfZP1KkHw36DB97+EumOCMI7iDeunTdaSKFoIeVlSjPr0oqB kA== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by aserp2120.oracle.com with ESMTP id 30b5um4ars-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 14 Apr 2020 12:35:38 +0000 Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 03ECHj6A180506; Tue, 14 Apr 2020 12:33:37 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userp3030.oracle.com with ESMTP id 30bqchhs56-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 14 Apr 2020 12:33:37 +0000 Received: from abhmp0017.oracle.com (abhmp0017.oracle.com [141.146.116.23]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 03ECXZUY019294; Tue, 14 Apr 2020 12:33:35 GMT Received: from kadam (/41.57.98.10) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 14 Apr 2020 05:33:34 -0700 Date: Tue, 14 Apr 2020 15:33:26 +0300 From: Dan Carpenter To: Camylla Goncalves Cantanheide Cc: gregkh@linuxfoundation.org, navid.emamdoost@gmail.com, sylphrenadin@gmail.com, nishkadg.linux@gmail.com, stephen@brennan.io, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, lkcamp@lists.libreplanetbr.org Subject: Re: [PATCH 1/2] staging: rtl8192u: Refactoring setKey function Message-ID: <20200414123326.GG1163@kadam> References: <20200413030129.861-1-c.cantanheide@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200413030129.861-1-c.cantanheide@gmail.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9590 signatures=668686 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxlogscore=999 bulkscore=0 malwarescore=0 phishscore=0 mlxscore=0 spamscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2004140103 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9590 signatures=668686 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 clxscore=1011 bulkscore=0 mlxscore=0 mlxlogscore=999 lowpriorityscore=0 impostorscore=0 adultscore=0 phishscore=0 spamscore=0 suspectscore=0 malwarescore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2004140103 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 13, 2020 at 03:01:28AM +0000, Camylla Goncalves Cantanheide wrote: > Changes of the local variable value and > modification in the seletive repetition structure. > This changelog isn't totally clear why you're doing this. Just say: "I am refactorying setKey() to make it more clear. I have unrolled the first two iterations through the loop. This patch will not change runtime." So long as it's clear what you're trying to do and why, that's the important thing with a commit message. > Signed-off-by: Camylla Goncalves Cantanheide > --- > drivers/staging/rtl8192u/r8192U_core.c | 52 ++++++++++++-------------- > 1 file changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c > index 9b8d85a4855d..87c02aee3854 100644 > --- a/drivers/staging/rtl8192u/r8192U_core.c > +++ b/drivers/staging/rtl8192u/r8192U_core.c > @@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev) > void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype, > u8 *macaddr, u8 defaultkey, u32 *keycontent) > { > - u32 target_command = 0; > + u32 target_command = CAM_CONTENT_COUNT * entryno | BIT(31) | BIT(16); > u32 target_content = 0; > u16 us_config = 0; > u8 i; > @@ -4890,39 +4890,35 @@ void setKey(struct net_device *dev, u8 entryno, u8 keyindex, u16 keytype, > > RT_TRACE(COMP_SEC, > "====>to %s, dev:%p, EntryNo:%d, KeyIndex:%d, KeyType:%d, MacAddr%pM\n", > - __func__, dev, entryno, keyindex, keytype, macaddr); > + __func__, dev, entryno, keyindex, keytype, macaddr); Do this white space change in a separate patch. > > if (defaultkey) > us_config |= BIT(15) | (keytype << 2); > else > us_config |= BIT(15) | (keytype << 2) | keyindex; > > - for (i = 0; i < CAM_CONTENT_COUNT; i++) { > - target_command = i + CAM_CONTENT_COUNT * entryno; > - target_command |= BIT(31) | BIT(16); > - > - if (i == 0) { /* MAC|Config */ > - target_content = (u32)(*(macaddr + 0)) << 16 | > - (u32)(*(macaddr + 1)) << 24 | > - (u32)us_config; > - > - write_nic_dword(dev, WCAMI, target_content); > - write_nic_dword(dev, RWCAM, target_command); > - } else if (i == 1) { /* MAC */ > - target_content = (u32)(*(macaddr + 2)) | > - (u32)(*(macaddr + 3)) << 8 | > - (u32)(*(macaddr + 4)) << 16 | > - (u32)(*(macaddr + 5)) << 24; > - write_nic_dword(dev, WCAMI, target_content); > - write_nic_dword(dev, RWCAM, target_command); > - } else { > - /* Key Material */ > - if (keycontent) { > - write_nic_dword(dev, WCAMI, > - *(keycontent + i - 2)); > - write_nic_dword(dev, RWCAM, target_command); > - } > - } > + target_content = macaddr[0] << 16 | > + macaddr[0] << 24 | > + (u32)us_config; > + > + write_nic_dword(dev, WCAMI, target_content); > + write_nic_dword(dev, RWCAM, target_command++); > + > + /* MAC */ > + target_content = macaddr[2] | > + macaddr[3] << 8 | > + macaddr[4] << 16 | > + macaddr[5] << 24; > + write_nic_dword(dev, WCAMI, target_content); > + write_nic_dword(dev, RWCAM, target_command++); > + > + /* Key Material */ > + if (!keycontent) > + return; > + > + for (i = 2; i < CAM_CONTENT_COUNT; i++) { > + write_nic_dword(dev, WCAMI, *keycontent++); This code was wrong in the original as well, but now that I see the bug let's fix it. CAM_CONTENT_COUNT is 8. 8 - 2 = 6. We are writing 6 u32 variables to write_nic_dword(). But the *keycontent buffer only has 4 u32 variables so it is a buffer overflow. > + write_nic_dword(dev, RWCAM, target_command++); > } > } regards, dan carpenter