From patchwork Thu Jun 29 08:51:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dmitry Antipov X-Patchwork-Id: 13296792 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0991AEB64D9 for ; Thu, 29 Jun 2023 08:52:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232274AbjF2Iwn (ORCPT ); Thu, 29 Jun 2023 04:52:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56506 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232172AbjF2IwJ (ORCPT ); Thu, 29 Jun 2023 04:52:09 -0400 Received: from forward102b.mail.yandex.net (forward102b.mail.yandex.net [IPv6:2a02:6b8:c02:900:1:45:d181:d102]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DAEC335BD for ; Thu, 29 Jun 2023 01:51:50 -0700 (PDT) Received: from mail-nwsmtp-smtp-production-canary-88.sas.yp-c.yandex.net (mail-nwsmtp-smtp-production-canary-88.sas.yp-c.yandex.net [IPv6:2a02:6b8:c08:1d21:0:640:c9e4:0]) by forward102b.mail.yandex.net (Yandex) with ESMTP id 36D7960078; Thu, 29 Jun 2023 11:51:48 +0300 (MSK) Received: by mail-nwsmtp-smtp-production-canary-88.sas.yp-c.yandex.net (smtp/Yandex) with ESMTPSA id ipIxcZ1DSeA0-leAGuNpX; Thu, 29 Jun 2023 11:51:47 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1688028707; bh=vEu8vMjIbX7nXG+CG0tA3nYjyZt9E4ldyPBMZU2sSFk=; h=Cc:Message-ID:References:Date:In-Reply-To:Subject:To:From; b=Qx6vJajV1TR0IUJfVBNBCyleYIybb3t1/DM+kk2rvPNw/kqSIMwkNAcvFA6uyib/7 6GnJIX11dYikW5tugIUQNUOCB66oPGfeldmCTFA1H/7Z4qPNKaiJEuZom+OOuzb7JQ O9NJR0oRiMT0rEXMAGfgPNNWnmWQbWM+PZmKx4Uk= Authentication-Results: mail-nwsmtp-smtp-production-canary-88.sas.yp-c.yandex.net; dkim=pass header.i=@yandex.ru From: Dmitry Antipov To: Kalle Valo Cc: Brian Norris , linux-wireless@vger.kernel.org, Dmitry Antipov Subject: [PATCH 2/3] [v4] wifi: mwifiex: fix fortify warning Date: Thu, 29 Jun 2023 11:51:01 +0300 Message-ID: <20230629085115.180499-2-dmantipov@yandex.ru> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230629085115.180499-1-dmantipov@yandex.ru> References: <20230629085115.180499-1-dmantipov@yandex.ru> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org When compiling with gcc 13.1 and CONFIG_FORTIFY_SOURCE=y, I've noticed the following: In function ‘fortify_memcpy_chk’, inlined from ‘mwifiex_construct_tdls_action_frame’ at drivers/net/wireless/marvell/mwifiex/tdls.c:765:3, inlined from ‘mwifiex_send_tdls_action_frame’ at drivers/net/wireless/marvell/mwifiex/tdls.c:856:6: ./include/linux/fortify-string.h:529:25: warning: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning] 529 | __read_overflow2_field(q_size_field, size); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The compiler actually complains on: memmove(pos + ETH_ALEN, &mgmt->u.action.category, sizeof(mgmt->u.action.u.tdls_discover_resp)); and it happens because the fortification logic interprets this as an attempt to overread 1-byte 'u.action.category' member of 'struct ieee80211_mgmt'. To silence this warning, it's enough to pass an address of 'u.action' itself instead of an address of its first member. This also fixes an improper usage of 'sizeof()'. Since 'skb' is extended with 'sizeof(mgmt->u.action.u.tdls_discover_resp) + 1' bytes (where 1 is actually 'sizeof(mgmt->u.action.category)'), I assume that the same number of bytes should be copied. Suggested-by: Brian Norris Signed-off-by: Dmitry Antipov Reviewed-by: Brian Norris --- v4: fix memmove() size calculation (Brian Norris) --- drivers/net/wireless/marvell/mwifiex/tdls.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c b/drivers/net/wireless/marvell/mwifiex/tdls.c index 97bb87c3676b..6c60621b6ccc 100644 --- a/drivers/net/wireless/marvell/mwifiex/tdls.c +++ b/drivers/net/wireless/marvell/mwifiex/tdls.c @@ -735,6 +735,7 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv, int ret; u16 capab; struct ieee80211_ht_cap *ht_cap; + unsigned int extra; u8 radio, *pos; capab = priv->curr_bss_params.bss_descriptor.cap_info_bitmap; @@ -753,7 +754,10 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv, switch (action_code) { case WLAN_PUB_ACTION_TDLS_DISCOVER_RES: - skb_put(skb, sizeof(mgmt->u.action.u.tdls_discover_resp) + 1); + /* See the layout of 'struct ieee80211_mgmt'. */ + extra = sizeof(mgmt->u.action.u.tdls_discover_resp) + + sizeof(mgmt->u.action.category); + skb_put(skb, extra); mgmt->u.action.category = WLAN_CATEGORY_PUBLIC; mgmt->u.action.u.tdls_discover_resp.action_code = WLAN_PUB_ACTION_TDLS_DISCOVER_RES; @@ -762,8 +766,7 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv, mgmt->u.action.u.tdls_discover_resp.capability = cpu_to_le16(capab); /* move back for addr4 */ - memmove(pos + ETH_ALEN, &mgmt->u.action.category, - sizeof(mgmt->u.action.u.tdls_discover_resp)); + memmove(pos + ETH_ALEN, &mgmt->u.action, extra); /* init address 4 */ eth_broadcast_addr(pos);