Message ID | 202212261903245548969@zte.com.cn |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp857780wrt; Mon, 26 Dec 2022 03:17:59 -0800 (PST) X-Google-Smtp-Source: AMrXdXucGXfYhZaTuuN7Ira/HsEhWtbEf6mEy3pmpQHX1ZfvDVRT+e/Wojp8+KLU2RKdAvzcn85c X-Received: by 2002:a17:907:d38c:b0:7c0:b0f9:e360 with SMTP id vh12-20020a170907d38c00b007c0b0f9e360mr14826163ejc.16.1672053478830; Mon, 26 Dec 2022 03:17:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672053478; cv=none; d=google.com; s=arc-20160816; b=R2cssZlPU5WjMyiXOKF8TyuLuQpxzfDts1NTK7htskaybWjjsR8Kp6DCbLDdpm4MrP f+W2Iw+OjsshOpc4HotklnWvm7B7ZzMKWwfoyWhKGgQegNsgUx98x20DTWstl4E2Cdgz DSQng0brZ9R1kVhkIGcLJvQxYNST3aFHLw6T8AZkyWtVnkJ1G9C3xTpuDnrp2v5AV3yz WybD3/Q+EzlcyYZJoyrB9dfocad/0Y78ruFv8rNNGA3WT9aIwB7mInQzOmte+94NNUnw wbtcoBtlKjzFywcMYoi5UswGv7PV0IKDJX8gYmMEspPlEdi2Bpb0CEKvXOU3AQAymmjf soNg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:subject:cc:to:from:mime-version:message-id:date; bh=s4RWseODXaQ94oB1t0k4+Gop2LzPB8iP87UtvgWvp7o=; b=RRK/Vsh9Q2pgvo03w/V6uPKNZZ4yw0xT7qjusiPeiZnEsbAI70gzsUe52C8LAGV3UC JHwtPdqL5A2RKGxYzAmAx5SMP9UjROyQyB16/Cv5wyPBUP2otpFJIOw5xj6NEA+wldMu qsdLHadwZwTdmV9x1udzvsBv0H9kdB3mMkepsa2YgfHIcMwJpSN1NIOxMYUQ0V3MxojH CuFrKpcSd893BuztEISzMiMbD02xpYw+AVok3k315dxv/2SZVpOhcBpqXtFjUMLPUVeS Wc5CmYOoNNNjVBb8xLxgV+seBztLgpnKKbokgNTAUePv36eZkY3A5oQgEBDdbT5K+0ee r5MA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=zte.com.cn Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id gb27-20020a170907961b00b007c0f2d1f654si8845994ejc.20.2022.12.26.03.17.34; Mon, 26 Dec 2022 03:17:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=zte.com.cn Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231716AbiLZLDd (ORCPT <rfc822;eddaouddi.ayoub@gmail.com> + 99 others); Mon, 26 Dec 2022 06:03:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53522 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229595AbiLZLDc (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 26 Dec 2022 06:03:32 -0500 Received: from mxhk.zte.com.cn (mxhk.zte.com.cn [63.216.63.40]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 46EBD111A for <linux-kernel@vger.kernel.org>; Mon, 26 Dec 2022 03:03:31 -0800 (PST) Received: from mse-fl2.zte.com.cn (unknown [10.5.228.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mxhk.zte.com.cn (FangMail) with ESMTPS id 4NgZdt03fKz8QrkZ; Mon, 26 Dec 2022 19:03:30 +0800 (CST) Received: from szxlzmapp02.zte.com.cn ([10.5.231.79]) by mse-fl2.zte.com.cn with SMTP id 2BQB3Ldg067680; Mon, 26 Dec 2022 19:03:21 +0800 (+08) (envelope-from yang.yang29@zte.com.cn) Received: from mapi (szxlzmapp01[null]) by mapi (Zmail) with MAPI id mid14; Mon, 26 Dec 2022 19:03:24 +0800 (CST) Date: Mon, 26 Dec 2022 19:03:24 +0800 (CST) X-Zmail-TransId: 2b0363a97f7cffffffffc5a1085d X-Mailer: Zmail v1.0 Message-ID: <202212261903245548969@zte.com.cn> Mime-Version: 1.0 From: <yang.yang29@zte.com.cn> To: <gustavoars@kernel.org> Cc: <linux-staging@lists.linux.dev>, <linux-kernel@vger.kernel.org>, <yang.yang29@zte.com.cn>, <xu.panda@zte.com.cn> Subject: =?utf-8?q?=5BPATCH_linux-next=5D_staging=3A_ks7010=3A_use_strscpy?= =?utf-8?q?=28=29_to_instead_of_strncpy=28=29?= Content-Type: text/plain; charset="UTF-8" X-MAIL: mse-fl2.zte.com.cn 2BQB3Ldg067680 X-Fangmail-Gw-Spam-Type: 0 X-FangMail-Miltered: at cgslv5.04-192.168.250.137.novalocal with ID 63A97F82.000 by FangMail milter! X-FangMail-Envelope: 1672052610/4NgZdt03fKz8QrkZ/63A97F82.000/10.5.228.133/[10.5.228.133]/mse-fl2.zte.com.cn/<yang.yang29@zte.com.cn> X-Fangmail-Anti-Spam-Filtered: true X-Fangmail-MID-QID: 63A97F82.000/4NgZdt03fKz8QrkZ X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_MSPIKE_H2, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1753275148534368314?= X-GMAIL-MSGID: =?utf-8?q?1753275148534368314?= |
Series |
[linux-next] staging: ks7010: use strscpy() to instead of strncpy()
|
|
Commit Message
Yang Yang
Dec. 26, 2022, 11:03 a.m. UTC
From: Xu Panda <xu.panda@zte.com.cn> The implementation of strscpy() is more robust and safer. That's now the recommended way to copy NUL-terminated strings. Signed-off-by: Xu Panda <xu.panda@zte.com.cn> Signed-off-by: Yang Yang <yang.yang29@zte.com> --- drivers/staging/ks7010/ks_wlan_net.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Comments
On Mon, Dec 26, 2022 at 07:03:24PM +0800, yang.yang29@zte.com.cn wrote: > From: Xu Panda <xu.panda@zte.com.cn> > > The implementation of strscpy() is more robust and safer. > That's now the recommended way to copy NUL-terminated strings. > > Signed-off-by: Xu Panda <xu.panda@zte.com.cn> > Signed-off-by: Yang Yang <yang.yang29@zte.com> > --- > drivers/staging/ks7010/ks_wlan_net.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c > index 044c807ca022..e03c87f0bfe7 100644 > --- a/drivers/staging/ks7010/ks_wlan_net.c > +++ b/drivers/staging/ks7010/ks_wlan_net.c > @@ -382,8 +382,7 @@ static int ks_wlan_get_nick(struct net_device *dev, > return -EPERM; > > /* for SLEEP MODE */ > - strncpy(extra, priv->nick, 16); > - extra[16] = '\0'; > + strscpy(extra, priv->nick, 17); I think this code is a buffer overflow. This is an implementation of SIOCGIWNICKN. net/wireless/wext-core.c 169 [IW_IOCTL_IDX(SIOCGIWNICKN)] = { 170 .header_type = IW_HEADER_TYPE_POINT, 171 .token_size = 1, 172 .max_tokens = IW_ESSID_MAX_SIZE, 173 }, As you can see there is a .max_tokens but no .min_tokens. It is called from ioctl_standard_iw_point(). So if the user specifies something smaller than 17 it leads to a buffer overflow. Your patch is mechanically correct, but now that we have eyes on the code we should fix the security bug instead making checkpatch happy or whatever. regards, dan carpenter
On Thu, Dec 29, 2022 at 12:45:10PM +0300, Dan Carpenter wrote: > On Mon, Dec 26, 2022 at 07:03:24PM +0800, yang.yang29@zte.com.cn wrote: > > From: Xu Panda <xu.panda@zte.com.cn> > > > > The implementation of strscpy() is more robust and safer. > > That's now the recommended way to copy NUL-terminated strings. > > > > Signed-off-by: Xu Panda <xu.panda@zte.com.cn> > > Signed-off-by: Yang Yang <yang.yang29@zte.com> > > --- > > drivers/staging/ks7010/ks_wlan_net.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c > > index 044c807ca022..e03c87f0bfe7 100644 > > --- a/drivers/staging/ks7010/ks_wlan_net.c > > +++ b/drivers/staging/ks7010/ks_wlan_net.c > > @@ -382,8 +382,7 @@ static int ks_wlan_get_nick(struct net_device *dev, > > return -EPERM; > > > > /* for SLEEP MODE */ > > - strncpy(extra, priv->nick, 16); > > - extra[16] = '\0'; > > + strscpy(extra, priv->nick, 17); > > I think this code is a buffer overflow. This is an implementation of > SIOCGIWNICKN. Huh... Maybe I'm wrong. I looked at a couple other implementations of SIOCGIWNICKN and they all seem to assume a 17 character buffer... Let me look deeper. I guess for now assume I am wrong. regards, dan carpenter
On Thu, Dec 29, 2022 at 12:45:10PM +0300, Dan Carpenter wrote: > On Mon, Dec 26, 2022 at 07:03:24PM +0800, yang.yang29@zte.com.cn wrote: > > From: Xu Panda <xu.panda@zte.com.cn> > > > > The implementation of strscpy() is more robust and safer. > > That's now the recommended way to copy NUL-terminated strings. > > > > Signed-off-by: Xu Panda <xu.panda@zte.com.cn> > > Signed-off-by: Yang Yang <yang.yang29@zte.com> > > --- > > drivers/staging/ks7010/ks_wlan_net.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c > > index 044c807ca022..e03c87f0bfe7 100644 > > --- a/drivers/staging/ks7010/ks_wlan_net.c > > +++ b/drivers/staging/ks7010/ks_wlan_net.c > > @@ -382,8 +382,7 @@ static int ks_wlan_get_nick(struct net_device *dev, > > return -EPERM; > > > > /* for SLEEP MODE */ > > - strncpy(extra, priv->nick, 16); > > - extra[16] = '\0'; > > + strscpy(extra, priv->nick, 17); > > I think this code is a buffer overflow. This is an implementation of > SIOCGIWNICKN. > > net/wireless/wext-core.c > 169 [IW_IOCTL_IDX(SIOCGIWNICKN)] = { > 170 .header_type = IW_HEADER_TYPE_POINT, > 171 .token_size = 1, > 172 .max_tokens = IW_ESSID_MAX_SIZE, > 173 }, > Yeah. I was wrong. The extra size here is .max_tokens * .token_size so it's 32. Sorry for the noise! Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> regards, dan carpenter
On Thu, Dec 29, 2022 at 12:54:28PM +0300, Dan Carpenter wrote: > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> Heh. My fingers are on autopilot. Reviewed-by: Dan Carpenter <error27@gmail.com> regards, dan carpenter
diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index 044c807ca022..e03c87f0bfe7 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -382,8 +382,7 @@ static int ks_wlan_get_nick(struct net_device *dev, return -EPERM; /* for SLEEP MODE */ - strncpy(extra, priv->nick, 16); - extra[16] = '\0'; + strscpy(extra, priv->nick, 17); dwrq->data.length = strlen(extra) + 1; return 0;