Re: [PATCH 4/4] samples/cgroup: don't let dirname() clobber the monitored path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] samples/cgroup: main() prints dirname()-clobbered path instead of argv[1]
--

commit e427c2a6d742e00fa3e83aac4c0262cf23a479ec
Author: Song Hu <husong@kylinos.cn>

samples/cgroup: don't let dirname() clobber the monitored path

This commit duplicates argv[1] with strdup() before passing it to dirname()
in cgroup_event_listener so that dirname() does not modify argv[1] in place.

> diff --git a/samples/cgroup/cgroup_event_listener.c b/samples/cgroup/cgroup_event_listener.c
> index 3d70dc831a766..308cacec80a8b 100644
> --- a/samples/cgroup/cgroup_event_listener.c
> +++ b/samples/cgroup/cgroup_event_listener.c
> @@ -26,17 +26,27 @@ int main(int argc, char **argv)
>  	int event_control = -1;
>  	char event_control_path[PATH_MAX];
>  	char line[LINE_MAX];
> +	char *path;
>  	int ret;
>  
>  	if (argc != 3)
>  		errx(1, "%s", USAGE_STR);
>  
> -	cfd = open(argv[1], O_RDONLY);
> +	/*
> +	 * dirname(3) may modify its argument in place; work on a copy so
> +	 * argv[1] still refers to the monitored file when events are
> +	 * reported.
> +	 */
> +	path = strdup(argv[1]);
> +	if (!path)
> +		err(1, "Cannot duplicate %s", argv[1]);
> +
> +	cfd = open(path, O_RDONLY);
>  	if (cfd == -1)
> -		err(1, "Cannot open %s", argv[1]);
> +		err(1, "Cannot open %s", path);
>  
>  	ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control",
> -			dirname(argv[1]));
> +			dirname(path));
>  	if (ret >= PATH_MAX)
>  		errx(1, "Path to cgroup.event_control is too long");
>  
> @@ -76,7 +86,7 @@ int main(int argc, char **argv)
>  		if (ret == -1)
>  			err(1, "cgroup.event_control is not accessible any more");
>  
> -		printf("%s %s: crossed\n", argv[1], argv[2]);
> +		printf("%s %s: crossed\n", path, argv[2]);

[Severity: Medium]
Should printf() in main() keep printing argv[1] here instead of path?

dirname(path) modifies the buffer pointed to by path in place while leaving
argv[1] intact, so switching the printf() call from argv[1] to path causes
main() to print the truncated parent directory rather than the full monitored
file path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102122528.1498193-1-husong@kylinos.cn?part=4