public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Alexander Lobakin <aleksander.lobakin@intel.com>
Cc: Kees Cook <kees@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nicolas Schier <nicolas.schier@linux.dev>,
	linux-kbuild@vger.kernel.org,
	Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2] kbuild: Re-enable -Wunterminated-string-initialization
Date: Tue, 5 Aug 2025 14:48:23 -0700	[thread overview]
Message-ID: <20250805214823.GB200407@ax162> (raw)
In-Reply-To: <e4d801e3-3004-484b-897d-ed43c25e1576@intel.com>

On Tue, Aug 05, 2025 at 04:50:28PM +0200, Alexander Lobakin wrote:
> From: Nathan Chancellor <nathan@kernel.org>
> Date: Sun, 3 Aug 2025 10:32:35 -0700
> 
> > On Sat, Aug 02, 2025 at 11:43:32AM -0700, Kees Cook wrote:
> >> With the few remaining fixes now landed, we can re-enable the option
> >> -Wunterminated-string-initialization (via -Wextra). Both GCC and Clang
> >> have the required multi-dimensional nonstring attribute support.
> 
> [...]
> 
> > diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
> > index 55a1a96cd834..05d4323c6a13 100644
> > --- a/drivers/net/ethernet/ti/netcp_ethss.c
> > +++ b/drivers/net/ethernet/ti/netcp_ethss.c
> > @@ -771,7 +771,7 @@ static struct netcp_module xgbe_module;
> >  
> >  /* Statistic management */
> >  struct netcp_ethtool_stat {
> > -	char desc[ETH_GSTRING_LEN];
> > +	char desc[ETH_GSTRING_LEN] __nonstring;
> 
> 
> Hmmm, ETH_GSTRING_LEN is the maximum length of the driver's statistics
> name to be reported to Ethtool and this *includes* \0 at the end.
> If this compilation flag triggers a warning here, the driver devs need
> to fix their code. There should always be \0 at the end, `desc` is a
> "proper" C 0-terminated string.

Ack, I had misunderstood a previous fix that Kees did for a similar but
different instance of the warning in another Ethernet driver and I
did not look much further than the driver copying these values around
with memcpy(). This does trigger a warning, from the original message:

  drivers/net/ethernet/ti/netcp_ethss.c:1049:2: error: initializer-string for character array is too long, array size is 32 but initializer has size 33 (including the null terminating character); did you mean to use the 'nonstring' attribute? [-Werror,-Wunterminated-string-initialization]
   1049 |         GBENU_STATS_HOST(ale_unknown_ucast_bytes),
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/net/ethernet/ti/netcp_ethss.c:956:2: note: expanded from macro 'GBENU_STATS_HOST'
    956 |         "GBE_HOST:"#field, GBENU_STATS0_MODULE,                 \
        |         ^~~~~~~~~~~~~~~~~
  drivers/net/ethernet/ti/netcp_ethss.c:1051:2: error: initializer-string for character array is too long, array size is 32 but initializer has size 33 (including the null terminating character); did you mean to use the 'nonstring' attribute? [-Werror,-Wunterminated-string-initialization]
   1051 |         GBENU_STATS_HOST(ale_unknown_mcast_bytes),
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/net/ethernet/ti/netcp_ethss.c:956:2: note: expanded from macro 'GBENU_STATS_HOST'
    956 |         "GBE_HOST:"#field, GBENU_STATS0_MODULE,                 \
        |         ^~~~~~~~~~~~~~~~~
  drivers/net/ethernet/ti/netcp_ethss.c:1053:2: error: initializer-string for character array is too long, array size is 32 but initializer has size 33 (including the null terminating character); did you mean to use the 'nonstring' attribute? [-Werror,-Wunterminated-string-initialization]
   1053 |         GBENU_STATS_HOST(ale_unknown_bcast_bytes),
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/net/ethernet/ti/netcp_ethss.c:956:2: note: expanded from macro 'GBENU_STATS_HOST'
    956 |         "GBE_HOST:"#field, GBENU_STATS0_MODULE,                 \
        |         ^~~~~~~~~~~~~~~~~

So it seems to me like this is a legitimate problem? Are these
descriptions expected to be stable once they are released or are we able
to adjust them? We could maybe shave an 'o' from 'unknown' to easily
resolve this without losing much in the way of quick visual processing.

Cheers,
Nathan

diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 55a1a96cd834..70590a04b6fd 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -493,12 +493,12 @@ struct gbenu_hw_stats {
 	u32	ale_vid_ingress_drop;
 	u32	ale_da_eq_sa_drop;
 	u32	__rsvd_0[3];
-	u32	ale_unknown_ucast;
-	u32	ale_unknown_ucast_bytes;
-	u32	ale_unknown_mcast;
-	u32	ale_unknown_mcast_bytes;
-	u32	ale_unknown_bcast;
-	u32	ale_unknown_bcast_bytes;
+	u32	ale_unknwn_ucast;
+	u32	ale_unknwn_ucast_bytes;
+	u32	ale_unknwn_mcast;
+	u32	ale_unknwn_mcast_bytes;
+	u32	ale_unknwn_bcast;
+	u32	ale_unknwn_bcast_bytes;
 	u32	ale_pol_match;
 	u32	ale_pol_match_red;		/* NU */
 	u32	ale_pol_match_yellow;		/* NU */
@@ -953,7 +953,7 @@ static const struct netcp_ethtool_stat gbe13_et_stats[] = {
 
 #define GBENU_STATS_HOST(field)					\
 {								\
-	"GBE_HOST:"#field, GBENU_STATS0_MODULE,			\
+	"GBE_HST:"#field, GBENU_STATS0_MODULE,			\
 	sizeof_field(struct gbenu_hw_stats, field),		\
 	offsetof(struct gbenu_hw_stats, field)			\
 }
@@ -1045,12 +1045,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_HOST(ale_rate_limit_drop),
 	GBENU_STATS_HOST(ale_vid_ingress_drop),
 	GBENU_STATS_HOST(ale_da_eq_sa_drop),
-	GBENU_STATS_HOST(ale_unknown_ucast),
-	GBENU_STATS_HOST(ale_unknown_ucast_bytes),
-	GBENU_STATS_HOST(ale_unknown_mcast),
-	GBENU_STATS_HOST(ale_unknown_mcast_bytes),
-	GBENU_STATS_HOST(ale_unknown_bcast),
-	GBENU_STATS_HOST(ale_unknown_bcast_bytes),
+	GBENU_STATS_HOST(ale_unknwn_ucast),
+	GBENU_STATS_HOST(ale_unknwn_ucast_bytes),
+	GBENU_STATS_HOST(ale_unknwn_mcast),
+	GBENU_STATS_HOST(ale_unknwn_mcast_bytes),
+	GBENU_STATS_HOST(ale_unknwn_bcast),
+	GBENU_STATS_HOST(ale_unknwn_bcast_bytes),
 	GBENU_STATS_HOST(ale_pol_match),
 	GBENU_STATS_HOST(ale_pol_match_red),
 	GBENU_STATS_HOST(ale_pol_match_yellow),
@@ -1111,12 +1111,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P1(ale_rate_limit_drop),
 	GBENU_STATS_P1(ale_vid_ingress_drop),
 	GBENU_STATS_P1(ale_da_eq_sa_drop),
-	GBENU_STATS_P1(ale_unknown_ucast),
-	GBENU_STATS_P1(ale_unknown_ucast_bytes),
-	GBENU_STATS_P1(ale_unknown_mcast),
-	GBENU_STATS_P1(ale_unknown_mcast_bytes),
-	GBENU_STATS_P1(ale_unknown_bcast),
-	GBENU_STATS_P1(ale_unknown_bcast_bytes),
+	GBENU_STATS_P1(ale_unknwn_ucast),
+	GBENU_STATS_P1(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P1(ale_unknwn_mcast),
+	GBENU_STATS_P1(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P1(ale_unknwn_bcast),
+	GBENU_STATS_P1(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P1(ale_pol_match),
 	GBENU_STATS_P1(ale_pol_match_red),
 	GBENU_STATS_P1(ale_pol_match_yellow),
@@ -1177,12 +1177,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P2(ale_rate_limit_drop),
 	GBENU_STATS_P2(ale_vid_ingress_drop),
 	GBENU_STATS_P2(ale_da_eq_sa_drop),
-	GBENU_STATS_P2(ale_unknown_ucast),
-	GBENU_STATS_P2(ale_unknown_ucast_bytes),
-	GBENU_STATS_P2(ale_unknown_mcast),
-	GBENU_STATS_P2(ale_unknown_mcast_bytes),
-	GBENU_STATS_P2(ale_unknown_bcast),
-	GBENU_STATS_P2(ale_unknown_bcast_bytes),
+	GBENU_STATS_P2(ale_unknwn_ucast),
+	GBENU_STATS_P2(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P2(ale_unknwn_mcast),
+	GBENU_STATS_P2(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P2(ale_unknwn_bcast),
+	GBENU_STATS_P2(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P2(ale_pol_match),
 	GBENU_STATS_P2(ale_pol_match_red),
 	GBENU_STATS_P2(ale_pol_match_yellow),
@@ -1243,12 +1243,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P3(ale_rate_limit_drop),
 	GBENU_STATS_P3(ale_vid_ingress_drop),
 	GBENU_STATS_P3(ale_da_eq_sa_drop),
-	GBENU_STATS_P3(ale_unknown_ucast),
-	GBENU_STATS_P3(ale_unknown_ucast_bytes),
-	GBENU_STATS_P3(ale_unknown_mcast),
-	GBENU_STATS_P3(ale_unknown_mcast_bytes),
-	GBENU_STATS_P3(ale_unknown_bcast),
-	GBENU_STATS_P3(ale_unknown_bcast_bytes),
+	GBENU_STATS_P3(ale_unknwn_ucast),
+	GBENU_STATS_P3(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P3(ale_unknwn_mcast),
+	GBENU_STATS_P3(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P3(ale_unknwn_bcast),
+	GBENU_STATS_P3(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P3(ale_pol_match),
 	GBENU_STATS_P3(ale_pol_match_red),
 	GBENU_STATS_P3(ale_pol_match_yellow),
@@ -1309,12 +1309,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P4(ale_rate_limit_drop),
 	GBENU_STATS_P4(ale_vid_ingress_drop),
 	GBENU_STATS_P4(ale_da_eq_sa_drop),
-	GBENU_STATS_P4(ale_unknown_ucast),
-	GBENU_STATS_P4(ale_unknown_ucast_bytes),
-	GBENU_STATS_P4(ale_unknown_mcast),
-	GBENU_STATS_P4(ale_unknown_mcast_bytes),
-	GBENU_STATS_P4(ale_unknown_bcast),
-	GBENU_STATS_P4(ale_unknown_bcast_bytes),
+	GBENU_STATS_P4(ale_unknwn_ucast),
+	GBENU_STATS_P4(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P4(ale_unknwn_mcast),
+	GBENU_STATS_P4(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P4(ale_unknwn_bcast),
+	GBENU_STATS_P4(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P4(ale_pol_match),
 	GBENU_STATS_P4(ale_pol_match_red),
 	GBENU_STATS_P4(ale_pol_match_yellow),
@@ -1375,12 +1375,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P5(ale_rate_limit_drop),
 	GBENU_STATS_P5(ale_vid_ingress_drop),
 	GBENU_STATS_P5(ale_da_eq_sa_drop),
-	GBENU_STATS_P5(ale_unknown_ucast),
-	GBENU_STATS_P5(ale_unknown_ucast_bytes),
-	GBENU_STATS_P5(ale_unknown_mcast),
-	GBENU_STATS_P5(ale_unknown_mcast_bytes),
-	GBENU_STATS_P5(ale_unknown_bcast),
-	GBENU_STATS_P5(ale_unknown_bcast_bytes),
+	GBENU_STATS_P5(ale_unknwn_ucast),
+	GBENU_STATS_P5(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P5(ale_unknwn_mcast),
+	GBENU_STATS_P5(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P5(ale_unknwn_bcast),
+	GBENU_STATS_P5(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P5(ale_pol_match),
 	GBENU_STATS_P5(ale_pol_match_red),
 	GBENU_STATS_P5(ale_pol_match_yellow),
@@ -1441,12 +1441,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P6(ale_rate_limit_drop),
 	GBENU_STATS_P6(ale_vid_ingress_drop),
 	GBENU_STATS_P6(ale_da_eq_sa_drop),
-	GBENU_STATS_P6(ale_unknown_ucast),
-	GBENU_STATS_P6(ale_unknown_ucast_bytes),
-	GBENU_STATS_P6(ale_unknown_mcast),
-	GBENU_STATS_P6(ale_unknown_mcast_bytes),
-	GBENU_STATS_P6(ale_unknown_bcast),
-	GBENU_STATS_P6(ale_unknown_bcast_bytes),
+	GBENU_STATS_P6(ale_unknwn_ucast),
+	GBENU_STATS_P6(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P6(ale_unknwn_mcast),
+	GBENU_STATS_P6(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P6(ale_unknwn_bcast),
+	GBENU_STATS_P6(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P6(ale_pol_match),
 	GBENU_STATS_P6(ale_pol_match_red),
 	GBENU_STATS_P6(ale_pol_match_yellow),
@@ -1507,12 +1507,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P7(ale_rate_limit_drop),
 	GBENU_STATS_P7(ale_vid_ingress_drop),
 	GBENU_STATS_P7(ale_da_eq_sa_drop),
-	GBENU_STATS_P7(ale_unknown_ucast),
-	GBENU_STATS_P7(ale_unknown_ucast_bytes),
-	GBENU_STATS_P7(ale_unknown_mcast),
-	GBENU_STATS_P7(ale_unknown_mcast_bytes),
-	GBENU_STATS_P7(ale_unknown_bcast),
-	GBENU_STATS_P7(ale_unknown_bcast_bytes),
+	GBENU_STATS_P7(ale_unknwn_ucast),
+	GBENU_STATS_P7(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P7(ale_unknwn_mcast),
+	GBENU_STATS_P7(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P7(ale_unknwn_bcast),
+	GBENU_STATS_P7(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P7(ale_pol_match),
 	GBENU_STATS_P7(ale_pol_match_red),
 	GBENU_STATS_P7(ale_pol_match_yellow),
@@ -1573,12 +1573,12 @@ static const struct netcp_ethtool_stat gbenu_et_stats[] = {
 	GBENU_STATS_P8(ale_rate_limit_drop),
 	GBENU_STATS_P8(ale_vid_ingress_drop),
 	GBENU_STATS_P8(ale_da_eq_sa_drop),
-	GBENU_STATS_P8(ale_unknown_ucast),
-	GBENU_STATS_P8(ale_unknown_ucast_bytes),
-	GBENU_STATS_P8(ale_unknown_mcast),
-	GBENU_STATS_P8(ale_unknown_mcast_bytes),
-	GBENU_STATS_P8(ale_unknown_bcast),
-	GBENU_STATS_P8(ale_unknown_bcast_bytes),
+	GBENU_STATS_P8(ale_unknwn_ucast),
+	GBENU_STATS_P8(ale_unknwn_ucast_bytes),
+	GBENU_STATS_P8(ale_unknwn_mcast),
+	GBENU_STATS_P8(ale_unknwn_mcast_bytes),
+	GBENU_STATS_P8(ale_unknwn_bcast),
+	GBENU_STATS_P8(ale_unknwn_bcast_bytes),
 	GBENU_STATS_P8(ale_pol_match),
 	GBENU_STATS_P8(ale_pol_match_red),
 	GBENU_STATS_P8(ale_pol_match_yellow),

  reply	other threads:[~2025-08-05 21:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-02 18:43 [PATCH v2] kbuild: Re-enable -Wunterminated-string-initialization Kees Cook
2025-08-03 17:32 ` Nathan Chancellor
2025-08-05 14:50   ` Alexander Lobakin
2025-08-05 21:48     ` Nathan Chancellor [this message]
2025-08-06 15:29       ` Alexander Lobakin
2025-08-06 19:05         ` Kees Cook
2025-08-07 13:31           ` Alexander Lobakin
2025-08-07 22:00             ` Kees Cook
2025-08-11 14:25               ` Alexander Lobakin
2025-09-05 15:59       ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250805214823.GB200407@ax162 \
    --to=nathan@kernel.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=justinstitt@google.com \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=masahiroy@kernel.org \
    --cc=morbo@google.com \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=nicolas.schier@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox