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

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@redhat.com>
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@unknownbbqr.xyz>
> ---
>  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;

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.
> 
> Signed-off-by: unknownbbqrx <dev@unknownbbqr.xyz>

Contributions need to be attributed to real people using official
names. I'm going to re-send this patch with me as author and a
Suggested-by: unknownbbqrx <dev@unknownbbqr.xyz>, unless you answer
with an appropriate attribution (i.e. your real name) [1].

Thanks,
Gabriele

[1] -
https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

> ---
>  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;

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

[unknownbbqrx]

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.

Signed-off-by: unknownbbqrx <dev@unknownbbqr.xyz>
---
 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;
-- 
2.53.0