Received-SPF: pass (google.com: domain of linux-kernel+bounces-136514-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <da430107-23ff-4984-b6ff-2f2e94f1429d@intel.com>
Date: Tue, 9 Apr 2024 10:58:41 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3] net: dsa: lan9303: use ethtool_puts() for
 lan9303_get_strings()
To: Justin Stitt <justinstitt@google.com>
CC: Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, "Paolo
 Abeni" <pabeni@redhat.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-hardening@vger.kernel.org>, Kees Cook
	<keescook@chromium.org>
References: <20240408-strncpy-drivers-net-dsa-lan9303-core-c-v3-1-0c313694d25b@google.com>
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Content-Language: en-US
In-Reply-To: <20240408-strncpy-drivers-net-dsa-lan9303-core-c-v3-1-0c313694d25b@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bzMxeFZkTmNjUmlBOFk1ZzloK1Jpc0xBQjU2OUVsNWMwdHc1ejA3N2h3Q1Jm?=
 =?utf-8?B?d1puWnlJcnRZOTRwbXBlZWxjeHpCK3IvZ29JVjdyOXppNUhGNW1kZG9ERUxa?=
 =?utf-8?B?T29OR1QxWjA2REx4eDBDTExidU5HY2VjV1VQdlBsem05Qkk0T2tzcGF6aE1W?=
 =?utf-8?B?VmR1MW9zZDFReHBJRVNCTnZmck9Sam96aVVvbHdqSmtkZk1ySFFmMnI4SitY?=
 =?utf-8?B?Qk5mTkg0Nk9XSjFUekFGc3FBcDZEODRKT2VWUVZRbEhSbFgwelpLbUo4UkY0?=
 =?utf-8?B?Q0pGdkgxZkRHYjdHTzBLRndsWkR1enFNeWIxK0dKWGRha09BenFVSUJnV3lX?=
 =?utf-8?B?QlR0VFdXTUlBU29sMTEzU1hteW4yUTVha000K2JsMjQ3OU5mVFQzRndoUk14?=
 =?utf-8?B?c0hNUnJrUEwvTG1PZThuWUhJMnZjNEUrTUt1M0hvcFpNdmpNMU9pNUNXYmVJ?=
 =?utf-8?B?cjduUHhrbE5uS2d6cFNMdHhXY20wNy9OTk55aHZuME9OTXlIREphVkFmSlRZ?=
 =?utf-8?B?THd3SjgvbjBQTUVBYjI1cnBrZytMYlc0dWpzSVlDUldPYnhKa0tNakdWNERt?=
 =?utf-8?B?T21BVHlxVEd4eGhpQlZ1MVNEL2VmaEdoSHBmVG0zYVQ1UXBCRnBZNVpnOVRo?=
 =?utf-8?B?Qk4vVG9vTmVVYkV1c3RMWmV2aXNqQWd6cTI0R1RvcHdINVo4OTRla1doMW9T?=
 =?utf-8?B?bHhFcDZaR3JkbkVXbWUwZlZlNlI5Qzk5TWxNbU5RdkxCZkJtQjM1aGhJMXV3?=
 =?utf-8?B?aHhCbDdlclJFdEJtZUZZb256WnVuV2JoSW56U0g5bkhhdUJGL251bGNRbG5z?=
 =?utf-8?B?bXdDTHd4VjA3czkraWxTa1BhL09IV0VuYlhnNnNVN3ZFY3huK0MxMFRZVGh0?=
 =?utf-8?B?RlRXd21KdHRtZ2VldDZ3Q3J4NUdlQkMrL3JqaFJXU2pkNXZweDljM0c2YjY4?=
 =?utf-8?B?TmtldWRRaFYrMlh2Q2ZiUm45MDRzS1c2VWNDTmI2eTFybkFQNlhJbDMxTlVn?=
 =?utf-8?B?TW1vNXQxb1dYRDlWdlhOVzlyZWU1aDlRY0graDM4emNZUGNsME5mNUZzanVn?=
 =?utf-8?B?Y054aTR3bGxnQWprbDN5S0VSMEZrT1BzRkFTbDFrMlhSZGJsQXlKL2tOd3ds?=
 =?utf-8?B?ekMvd09rMXArVDB0L2NmN3cya29INTEwenZGMWY1dmdjcjhRcGwyeVRkWEpv?=
 =?utf-8?B?OXQ1UG1rRlU5M0tvY1VKY0QyUzlzemc3U29GUm9oWWRNUFAya0d0SGR3aEJ4?=
 =?utf-8?B?T1JkOXVOa2ttdndJL0FlZk0rcldiQ0twY0dLeFcwVEQ1UXFFQWc5YWZRNDFk?=
 =?utf-8?B?SjBjQTd5SkdpN2JkWS9PQkk3WFR1WHI3N2MyRWhMWHJoUkpRd0laZGlEQXNU?=
 =?utf-8?B?WmdyMTU0Y1RSL0N3cFhXbksxM05MWmZFWllWSWVDUHlsUlVXb3RxK0dpcVFm?=
 =?utf-8?B?QjJvcXFqZ1o2d29ZWGlIQmxteGVac08rV3ZXNTNuVENWM1FZa2dCSzRlSFYx?=
 =?utf-8?B?cmZMZjhMWTlocnNwaW5XalZRbnVpMGZ4L0Y4VUYrcnArZ1YwRDFYQTdpR1ho?=
 =?utf-8?B?Z2I5cjM1cythWXZtZ0t5Wnc4MzV0bTZHRzlJOEhyb2htTmZrN0ZWUmtpZWZs?=
 =?utf-8?B?NTFBNUNPMGU2NjhpRGwzd096WGQ3bG5ZRDVhM2pJNDBEcHRRUm5QVjF2Tm84?=
 =?utf-8?B?WWppSmRtd1pKUVRkSndJMXY4TnNCT0N4bHhYbE5PWFB3WC9CbDVCZ1V1WDNL?=
 =?utf-8?B?aDhOQVpLcFlOMjRIcTBHWVdrWlVYWTBVWHRIYWwwNVVzMW85YmkxZ0xmR1NS?=
 =?utf-8?B?ZG1ldXFacEI4ODBBWmVuR2JRWjBjdzdzSjRWb1NvT3I5aXRTbDBmNlB6bWNx?=
 =?utf-8?B?cDY4ejAxUllNUGk1aVJCU0lyNUplYzRVc1IyTEtERmpFSnZLcHpaSmFtY0xp?=
 =?utf-8?B?ZWl3bGYvdUtYMmtMOTBqNmxoS1E1YnVXQTAxbmtQSFpsOVBiNnhNWndkeHRY?=
 =?utf-8?B?b0JNN1QyYXJzN0tsc2VNKy9SVVNzM0YwQmVWcklsTjM4dHBEeURDL2pGQkUz?=
 =?utf-8?B?dDZQZ0ZNQlpQcGZtZlg0ekF1V0xLMlRXcUg2L0VtOEJSQTB2eDEvWTR1cDZU?=
 =?utf-8?B?WWFNQzYyZGgxcTFneU5OSGU1bDdScHUwVE01RTB1b0txTm5PYkgxRXROVER2?=
 =?utf-8?B?QUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3938a212-4da8-4e2e-c5a1-08dc5873891a
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB8718.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 09:00:42.6549
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mEhKWpq9RDMvgCNXlxX/HevO4OfxDx8IhCte1DotBEXP3BmUdFf52C7+F0jjnJQDlmTuU+hCLwA8hxu4rAYNLdczWEVxGVMW3Qxqj80KrP8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4847
X-OriginatorOrg: intel.com

From: Justin Stitt <justinstitt@google.com>
Date: Mon, 08 Apr 2024 21:01:57 +0000

> This pattern of strncpy with some pointer arithmetic setting fixed-sized
> intervals with string literal data is a bit weird so let's use
> ethtool_puts() as this has more obvious behavior and is less-error
> prone.
> 
> Nicely, we also get to drop a usage of the now deprecated strncpy() [1].
> 
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> Suggested-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> ---
> Changes in v3:
> - use ethtool_puts over ethtool_sprintf
> - Link to v2: https://lore.kernel.org/r/20231005-strncpy-drivers-net-dsa-lan9303-core-c-v2-1-feb452a532db@google.com
> 
> Changes in v2:
> - use ethtool_sprintf (thanks Alexander)
> - Link to v1: https://lore.kernel.org/r/20231005-strncpy-drivers-net-dsa-lan9303-core-c-v1-1-5a66c538147e@google.com
> ---
>  drivers/net/dsa/lan9303-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index fcb20eac332a..04d3af0eb28e 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1007,14 +1007,14 @@ static const struct lan9303_mib_desc lan9303_mib[] = {
>  static void lan9303_get_strings(struct dsa_switch *ds, int port,
>  				u32 stringset, uint8_t *data)
>  {
> +	u8 *buf = data;
>  	unsigned int u;
>  
>  	if (stringset != ETH_SS_STATS)
>  		return;
>  
>  	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> -		strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
> -			ETH_GSTRING_LEN);
> +		ethtool_puts(&buf, lan9303_mib[u].name);
>  	}

The braces { } around ethtool_puts() aren't needed anymore.

>  }
>  
> 
> ---
> base-commit: c85af715cac0a951eea97393378e84bb49384734
> change-id: 20231005-strncpy-drivers-net-dsa-lan9303-core-c-6386858e5c22
> 
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>

Thanks,
Olek