From patchwork Fri Jun 23 15:23:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 695707 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 D4845EB64D7 for ; Fri, 23 Jun 2023 15:25:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232418AbjFWPZN (ORCPT ); Fri, 23 Jun 2023 11:25:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42078 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232211AbjFWPZN (ORCPT ); Fri, 23 Jun 2023 11:25:13 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 313621BF2; Fri, 23 Jun 2023 08:25:12 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B7A2761AA0; Fri, 23 Jun 2023 15:25:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 730FFC433C8; Fri, 23 Jun 2023 15:25:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1687533911; bh=PWzWmQ+cBgJK6M6TA5MtMDmO0z6eE5AOkMWX2YVaO2k=; h=From:To:Cc:Subject:Date:From; b=P5hRm+79D+2sZ4kgfzfW91OywrM3XnOisdudNi2GlRMtyg5UH0fJKk2zIoCvyQc0P OwxUG8xC5hN6tuNUjuWTNRak3gq/OOfYS97/iZCng3xsnGxJBupj1nQL4aFhvctmTl augdY8OVO3DMh8cmyA8rxuEBFe/VRRgZQOTlmPsUAmYmcvKAUXHz+EIfDEAFO5yorG jOArBFZOJ+JaLhy4odrlwAyqdOyzmGfQemDAzuZZPVMFdSw6hqjMbX4XQiLJ8l/Zjj z3IgMXZdxB3oDlwbQ0XHp6Lf1srAPR+fS7bEartCAlVGgVq1o5ryKAW6kT7e8BK7jn CoDA9RnFTW+uA== From: Arnd Bergmann To: Christian Lamparter , Kalle Valo , Kees Cook , Johannes Berg Cc: Arnd Bergmann , linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] carl9170: re-fix fortified-memset warning Date: Fri, 23 Jun 2023 17:23:59 +0200 Message-Id: <20230623152443.2296825-1-arnd@kernel.org> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Arnd Bergmann The carl9170_tx_release() function sometimes triggers a fortified-memset warning in my randconfig builds: In file included from include/linux/string.h:254, from drivers/net/wireless/ath/carl9170/tx.c:40: In function 'fortify_memset_chk', inlined from 'carl9170_tx_release' at drivers/net/wireless/ath/carl9170/tx.c:283:2, inlined from 'kref_put' at include/linux/kref.h:65:3, inlined from 'carl9170_tx_put_skb' at drivers/net/wireless/ath/carl9170/tx.c:342:9: include/linux/fortify-string.h:493:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] 493 | __write_overflow_field(p_size_field, size); Kees previously tried to avoid this by using memset_after(), but it seems this does not fully address the problem. I noticed that the memset_after() here is done on a different part of the union (status) than the original cast was from (rate_driver_data), which may confuse the compiler. Unfortunately, the memset_after() trick does not work on driver_rates[] because that is part of an anonymous struct, and I could not get struct_group() to do this either. Using two separate memset() calls on the two members does address the warning though. Fixes: fb5f6a0e8063b ("mac80211: Use memset_after() to clear tx status") Signed-off-by: Arnd Bergmann Reviewed-by: Kees Cook --- drivers/net/wireless/ath/carl9170/tx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c index 6bb9aa2bfe654..88ef6e023f826 100644 --- a/drivers/net/wireless/ath/carl9170/tx.c +++ b/drivers/net/wireless/ath/carl9170/tx.c @@ -280,7 +280,8 @@ static void carl9170_tx_release(struct kref *ref) * carl9170_tx_fill_rateinfo() has filled the rate information * before we get to this point. */ - memset_after(&txinfo->status, 0, rates); + memset(&txinfo->pad, 0, sizeof(txinfo->pad)); + memset(&txinfo->rate_driver_data, 0, sizeof(txinfo->rate_driver_data)); if (atomic_read(&ar->tx_total_queued)) ar->tx_schedule = true; From patchwork Fri Jun 23 15:24:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 696812 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 7A398EB64DD for ; Fri, 23 Jun 2023 15:26:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229734AbjFWP0C (ORCPT ); Fri, 23 Jun 2023 11:26:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42788 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232532AbjFWPZy (ORCPT ); Fri, 23 Jun 2023 11:25:54 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 28E692129; Fri, 23 Jun 2023 08:25:47 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B4AE461A9D; Fri, 23 Jun 2023 15:25:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D02DBC433C0; Fri, 23 Jun 2023 15:25:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1687533946; bh=NvWgTGjsHRqDtvBYDx/2FZ7WiFRdFqxIOJ2GEgQdP5s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jjSV3khYn7DQtMsuDg9EoNpKhthnIYHAR3qxDpF7Arr09pVBiSIRwUoI5OImA8F3s qmlveYBP11wTA/VaTOeOK/7UPuOJpMlPgbxtyOfpAb7MCX8PGdeVw8AX2mWpyCMGL/ pkngpeZ4JGYxGMlzpLK1o860FrlWOZPNVFSi7u08r2V15VEqmBbkab00TOFCACUWYH qtq0Jf3f/vajbOgUJdUm+WzWV7Ycb5rkdLKbMJIkjBXxGD5sCB3mTP3mKs5YT28mI7 8dwmjH+dAP0xRobOCLzn27EAfNnGTez82/9NOZJ5i1Hx9WiBXLPIU4DNp17r5a4SQV 4BuY8MkMhjXJg== From: Arnd Bergmann To: Johannes Berg Cc: Arnd Bergmann , Christian Lamparter , Kalle Valo , Johannes Berg , Kees Cook , linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Gregory Greenman , Benjamin Berg , Ryder Lee , Ilan Peer , Felix Fietkau , Aloka Dixit , Avraham Stern , Emmanuel Grumbach , netdev@vger.kernel.org Subject: [PATCH 2/2] mac80211: make ieee80211_tx_info padding explicit Date: Fri, 23 Jun 2023 17:24:00 +0200 Message-Id: <20230623152443.2296825-2-arnd@kernel.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230623152443.2296825-1-arnd@kernel.org> References: <20230623152443.2296825-1-arnd@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Arnd Bergmann While looking at a bug, I got rather confused by the layout of the 'status' field in ieee80211_tx_info. Apparently, the intention is that status_driver_data[] is used for driver specific data, and fills up the size of the union to 40 bytes, just like the other ones. This is indeed what actually happens, but only because of the combination of two mistakes: - "void *status_driver_data[18 / sizeof(void *)];" is intended to be 18 bytes long but is actually two bytes shorter because of rounding-down in the division, to a multiple of the pointer size (4 bytes or 8 bytes). - The other fields combined are intended to be 22 bytes long, but are actually 24 bytes because of padding in front of the unaligned tx_time member, and in front of the pointer array. The two mistakes cancel out. so the size ends up fine, but it seems more helpful to make this explicit, by having a multiple of 8 bytes in the size calculation and explicitly describing the padding. Fixes: ea5907db2a9cc ("mac80211: fix struct ieee80211_tx_info size") Fixes: 02219b3abca59 ("mac80211: add WMM admission control support") Signed-off-by: Arnd Bergmann --- include/net/mac80211.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 3a8a2d2c58c38..ca4dc8a14f1bb 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1192,9 +1192,11 @@ struct ieee80211_tx_info { u8 ampdu_ack_len; u8 ampdu_len; u8 antenna; + u8 pad; u16 tx_time; u8 flags; - void *status_driver_data[18 / sizeof(void *)]; + u8 pad2; + void *status_driver_data[16 / sizeof(void *)]; } status; struct { struct ieee80211_tx_rate driver_rates[