Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Thu, 12 Oct 2023 17:05:19 +0200
From:   Louis Peens <louis.peens@corigine.com>
To:     Justin Stitt <justinstitt@google.com>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>, oss-drivers@corigine.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org
Subject: Re: [PATCH] nfp: replace deprecated strncpy with strscpy
Message-ID: <ZSgLLwDD9J3uDutU@LouisNoVo>
References: <20231011-strncpy-drivers-net-ethernet-netronome-nfp-nfpcore-nfp_resource-c-v1-1-7d1c984f0eba@google.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231011-strncpy-drivers-net-ethernet-netronome-nfp-nfpcore-nfp_resource-c-v1-1-7d1c984f0eba@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?64vkdV8kNrnRW+//kKx7FosAlonD6MqSeTigTDPdKCE+6OyFCvl89HyDSHRr?=
 =?us-ascii?Q?5qAPs2PBgnTRYwQSLpNSsnDPQJAlOGGbr407yVdGAjT7QRxC4YBZnYpJMPiD?=
 =?us-ascii?Q?8QfrGThlJOZZQBdjv0rnoq4JXJVjBxDh41ZaHtLDeMM4z01GUt/f/I0/lwN9?=
 =?us-ascii?Q?OPUIpWC44m+YZWPHDlp3D3wOOAuWydMpllyuUdDitZb8DtIuAXMuR+lZWLYS?=
 =?us-ascii?Q?fvIzLcCI2tnrCWpTubU4DaVttvJScAIQ4p6kPkEUNUN3/amI8T6HmEgWpXw8?=
 =?us-ascii?Q?47InZIHU/ZxUACONVo9VssCSKtztPzZ6/+sG6yWDgxkMxhUlNUiOmWzJlmr7?=
 =?us-ascii?Q?L01nAsdFtbkPr7k2S14EDnx/yWsrYyiig+Bqx8TVl3uhPaKXl9dmlYzQucNs?=
 =?us-ascii?Q?R8wAOcxDbMmOdoeaIql4vt5PT+21170VRapt8yihvi3eHcpJCTwS9TdT4K1I?=
 =?us-ascii?Q?1B6h/u+4IXJLOyb2mWjKYTM76RpR7NmFvsqqOHXfXKVAgo5Y1aygHqi2Ml3G?=
 =?us-ascii?Q?yIrqlolMb3mXqRYBCE1U+1Zg5OwR6Yzj42EgmriGPcd0e1Dw+ZzcEhZI+24z?=
 =?us-ascii?Q?BfuqsJIXNGeiMr0L+0Bd/HB96QIWfNt5lHvLhNJJi2v7g8jamOkKLVzqvMTL?=
 =?us-ascii?Q?7y1KSP8ONSSpulqcfbUivmB+M/xHaDkjGXPgMsmzGnS+B/EWnmK8GLCQoVfK?=
 =?us-ascii?Q?Fx5JP9PiB2n9gF015QrItB4WO9LgrOOZhT94380wTN9ppbSDSMVUnHv7J+/z?=
 =?us-ascii?Q?bs4qpIB5HP7hMFNPWpMUjBGMHxjFKnMSZ0pdU9h/XzBGE3SK9kUHYiqIJSPb?=
 =?us-ascii?Q?yRvuRlJBNqnqk5Rk6QpDUdoi898c7+mdlT3k/8gCzra9A2OBU1cA9Yc+TKCf?=
 =?us-ascii?Q?O+xXv2YOtvILAy6oNDUxBcyeZljEd9yebJMZ9EvcUCzzb4YQwTou6wg/WMPb?=
 =?us-ascii?Q?3gwo/RSh3uNvR4N7e1J1AkulD6t0faUF44q5ejGvoFjcf10QwhI+F/ROurie?=
 =?us-ascii?Q?6t58QmnTGb+BJ3t88LwKPAzsjRDRv2lME285Us+9UAOytkOuoV/ZO9bkLcid?=
 =?us-ascii?Q?FZqMi685lK2DlUe6zEaKP8K1D4pykL958bjEk3vWuon1/Gw05P/VbOopZrpz?=
 =?us-ascii?Q?BCaA5aEIx6cjXtuUReEgzRKFNlc2OHeympY6dzluAY8mx+ujwFd7z0hMZz+P?=
 =?us-ascii?Q?MY/7ZXr07d57idkwwZpyF2dmP4+2ApEUJVaIXufkhCAE7G7fJ/AMD7IaaWJG?=
 =?us-ascii?Q?Q8wHefI2AgZzMkDSz+4F+1l3/dUA4Mtvy1HP7qwoOyTQKm5IHR2amIOmMU3N?=
 =?us-ascii?Q?TGL3ooI0l8LWwq1vaEobBHrn478o0InqIYLBHhVwtSZG8rZSOaoVmua8osLG?=
 =?us-ascii?Q?JSdrfcybFvbGyWYMAwiMZJk5WBhEkjytzNmzUjdsiyT5HeXicqDZ+5htxPub?=
 =?us-ascii?Q?jCtdA65SnTJkGiEnQANagxbrdERqbxDBeFArvVrJlDWUhW5uybFqg+7umsGq?=
 =?us-ascii?Q?kT3zvGDlv8CGoI/WfRkJdC1CAwrZH9ocObG6xtO53Nj7H7aD80J+PDmIJ7GF?=
 =?us-ascii?Q?Wr5MCapaGAbyhre9OcesuRck4YF1eUV1L2fJk2jga0fFdCVafSV/tUyHbphB?=
 =?us-ascii?Q?ag=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 79c8bfb7-0c19-46e4-cf3b-08dbcb34afec
X-MS-Exchange-CrossTenant-AuthSource: DM6PR13MB4249.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Oct 2023 15:05:35.8072
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Uuki/S7+5OTgzF6JppymcTbPc4QzxtnE6qXunOYwDA8d6bkM5SKvMn/iU+H6RCLSgkMDSJAfvkp7Yu7ACgw2T6lpWqPXZWzRi6ohScpdlfg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR13MB6000
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,
	SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 12 Oct 2023 08:05:56 -0700 (PDT)

On Wed, Oct 11, 2023 at 09:48:39PM +0000, Justin Stitt wrote:
> strncpy() is deprecated for use on NUL-terminated destination strings
> [1] and as such we should prefer more robust and less ambiguous string
> interfaces.
> 
> We expect res->name to be NUL-terminated based on its usage with format
> strings:
> |       dev_err(cpp->dev.parent, "Dangling area: %d:%d:%d:0x%0llx-0x%0llx%s%s\n",
> |               NFP_CPP_ID_TARGET_of(res->cpp_id),
> |               NFP_CPP_ID_ACTION_of(res->cpp_id),
> |               NFP_CPP_ID_TOKEN_of(res->cpp_id),
> |               res->start, res->end,
> |               res->name ? " " : "",
> |               res->name ? res->name : "");
> ... and with strcmp()
> |       if (!strcmp(res->name, NFP_RESOURCE_TBL_NAME)) {
> 
> Moreover, NUL-padding is not required as `res` is already
> zero-allocated:
> |       res = kzalloc(sizeof(*res), GFP_KERNEL);
> 
> Considering the above, a suitable replacement is `strscpy` [2] due to
> the fact that it guarantees NUL-termination on the destination buffer
> without unnecessarily NUL-padding.
> 
> Let's also opt to use the more idiomatic strscpy() usage of (dest, src,
> sizeof(dest)) rather than (dest, src, SOME_LEN).
> 
> Typically the pattern of 1) allocate memory for string, 2) copy string
> into freshly-allocated memory is a candidate for kmemdup_nul() but in
> this case we are allocating the entirety of the `res` struct and that
> should stay as is. As mentioned above, simple 1:1 replacement of strncpy
> -> strscpy :)
> 
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Note: build-tested only.
> 
> Found with: $ rg "strncpy\("
Thanks Justin, I did also now check it on a nfp.

Acked-by: Louis Peens <louis.peens@corigine.com>
> ---
>  drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
> index ce7492a6a98f..279ea0b56955 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
> @@ -159,7 +159,7 @@ nfp_resource_acquire(struct nfp_cpp *cpp, const char *name)
>  	if (!res)
>  		return ERR_PTR(-ENOMEM);
>  
> -	strncpy(res->name, name, NFP_RESOURCE_ENTRY_NAME_SZ);
> +	strscpy(res->name, name, sizeof(res->name));
>  
>  	dev_mutex = nfp_cpp_mutex_alloc(cpp, NFP_RESOURCE_TBL_TARGET,
>  					NFP_RESOURCE_TBL_BASE,
> 
> ---
> base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
> change-id: 20231011-strncpy-drivers-net-ethernet-netronome-nfp-nfpcore-nfp_resource-c-1812b8357fcd
> 
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
>