Re: [PATCH 1/1] tools/rv: ensure monitor name and desc are NUL-terminated

[Gabriele Monaco]

On Thu, 2026-04-23 at 17:19 +0300, unknownbbqrx wrote:
> 
> ikm_fill_monitor_definition() copies monitor name and description
> with
> strncpy(), but does not guarantee NUL termination when source strings
> are
> equal to or longer than the destination buffers.
> 
> Clamp copies to sizeof(dst) - 1 and explicitly append '\0' for both
> fields
> to keep them safe for later string operations.

Hi,

thanks for the fix!
Looks good to me.

Reviewed-by: Gabriele Monaco <gmonaco@xxxxxxxxxx>
Fixes: 6d60f89691fc9 ("tools/rv: Add in-kernel monitor interface")

On a side note, you sent 2 patches and you apparently sent them both
twice (did you issue git send-email twice? They seem equivalent to me),
next time you could merge them in the same series, just preparing them
in the same branch and passing them all to git format-patch/send-email
[1]. In general you'd also add a cover letter, can be very simple in
this case.
That's usually tidier and easier to apply for maintainers/reviewers.
(You can ignore it this time)

Also add the Fixes: tag if you're fixing something (e.g. a potential
buffer overflow in this case), I did it for you now but you can find
the commit you're fixing using git blame.

[1] -
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> 
> Signed-off-by: unknownbbqrx <dev@xxxxxxxxxxxxxxx>
> ---
>  tools/verification/rv/src/in_kernel.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/verification/rv/src/in_kernel.c
> b/tools/verification/rv/src/in_kernel.c
> index 4bb746ea6..d32453824 100644
> --- a/tools/verification/rv/src/in_kernel.c
> +++ b/tools/verification/rv/src/in_kernel.c
> @@ -215,10 +215,11 @@ static int ikm_fill_monitor_definition(char
> *name, struct monitor *ikm, char *co
>  		return -1;
>  	}
>  
> -	strncpy(ikm->name, nested_name, MAX_DA_NAME_LEN);
> +	strncpy(ikm->name, nested_name, sizeof(ikm->name) - 1);
> +	ikm->name[sizeof(ikm->name) - 1] = '\0';
>  	ikm->enabled = enabled;
> -	strncpy(ikm->desc, desc, MAX_DESCRIPTION);
> -
> +	strncpy(ikm->desc, desc, sizeof(ikm->desc) - 1);
> +	ikm->desc[sizeof(ikm->desc) - 1] = '\0';
>  	free(desc);
>  
>  	return 0;