Re: [PATCH v2] media: verisilicon: Replace strlcat() with scnprintf()

From: Kees Cook <kees@kernel.org>
To: Mariia Nikitash <nikitash.mariiaw@gmail.com>
Cc: nicolas.dufresne@collabora.com, benjamin.gaignard@collabora.com,
	p.zabel@pengutronix.de, mchehab@kernel.org,
	hverkuil+cisco@kernel.org, linux-media@vger.kernel.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org, justinstitt@google.com,
	morbo@google.com, david.laight.linux@gmail.com
Subject: Re: [PATCH v2] media: verisilicon: Replace strlcat() with scnprintf()
Date: Thu, 12 Nov 2026 09:42:03 -0800	[thread overview]
Message-ID: <202611120937.C289DA9C4D@keescook> (raw)
In-Reply-To: <20261111032446.29803-1-nikitash.mariiaw@gmail.com>

On Tue, Nov 10, 2026 at 07:24:46PM -0800, Mariia Nikitash wrote:
> In preparation for removing the deprecated strlcat() API [1], replace the
> strscpy() and strlcat() calls in hantro_add_func() with a single
> scnprintf() call.
> 
> This preserves the resulting string, including NUL termination
> and truncation behavior.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>
> ---
> Changes in v2:
> - Replace seq_buf with a single scnprintf() call, as suggested
>   by Hans Verkuil.

Reviewed-by: Kees Cook <kees@kernel.org>

I think the Subject prefix is fine, it matches the prior poking of this
area:

b66215e7b780 ("media: verisilicon: replace snprintf with strscpy+strlcat")

For vfd->name,

	struct video_device {
		...
		        char name[64];

Does this need to be raised?

Because match->compatible is quite large:

struct of_device_id {
...
        char    compatible[128];

The real question remains: how should truncation of vfd->name (by
anything that constructs it) be handled?

-Kees

> 
>  drivers/media/platform/verisilicon/hantro_drv.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 26e5d5547466..d8854ceb4fed 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -902,9 +902,8 @@ static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid)
>  	vfd->vfl_dir = VFL_DIR_M2M;
>  	vfd->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M_MPLANE;
>  	vfd->ioctl_ops = &hantro_ioctl_ops;
> -	strscpy(vfd->name, match->compatible, sizeof(vfd->name));
> -	strlcat(vfd->name, funcid == MEDIA_ENT_F_PROC_VIDEO_ENCODER ?
> -		"-enc" : "-dec", sizeof(vfd->name));
> +	scnprintf(vfd->name, sizeof(vfd->name), "%s%s", match->compatible,
> +		  funcid == MEDIA_ENT_F_PROC_VIDEO_ENCODER ? "-enc" : "-dec");
>  
>  	if (funcid == MEDIA_ENT_F_PROC_VIDEO_ENCODER) {
>  		vpu->encoder = func;
> -- 
> 2.43.0
> 

-- 
Kees Cook