linux-kernel - Re: [PATCH] cpupower: Make help command available for custom install dir

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6d745d8d-33b1-4aed-b9c5-095073bc8cde@linuxfoundation.org>
Date: Fri, 21 Jun 2024 09:02:47 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Roman Storozhenko <romeusmeister@...il.com>,
 Thomas Renninger <trenn@...e.com>, Shuah Khan <shuah@...nel.org>
Cc: Javier Carrasco <javier.carrasco.cruz@...il.com>,
 linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH] cpupower: Make help command available for custom install
 dir

On 6/21/24 02:13, Roman Storozhenko wrote:
> When the 'cpupower' utility installed in the custom dir, it fails to
> render appopriate help info for a particular subcommand:

appopriate -> appropriate
Spell check the commit message.

> $ LD_LIBRARY_PATH=lib64/ bin/cpupower help monitor
> with error message like 'No manual entry for cpupower-monitor.1'
> The issue is that under the hood it calls 'exec' function with
> the following args: 'man cpupower-monitor.1'. In turn, 'man' search
> path is defined in '/etc/manpath.config'. Of course it contains only
> standard system man paths.
> Make subcommands man pages available for user using the following rule:
> Render a man page if it is installed in the custom install dir, otherwise
> allow man to search this page by name system-wide as a last resort.
> 

Good find.

> Signed-off-by: Roman Storozhenko <romeusmeister@...il.com>
> ---
>   tools/power/cpupower/utils/cpupower.c | 54 ++++++++++++++++++++++++++++++-----
>   1 file changed, 47 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpupower.c b/tools/power/cpupower/utils/cpupower.c
> index 9ec973165af1..da4bc6de7494 100644
> --- a/tools/power/cpupower/utils/cpupower.c
> +++ b/tools/power/cpupower/utils/cpupower.c
> @@ -12,6 +12,8 @@
>   #include <unistd.h>
>   #include <errno.h>
>   #include <sched.h>
> +#include <libgen.h>
> +#include <limits.h>
>   #include <sys/types.h>
>   #include <sys/stat.h>
>   #include <sys/utsname.h>
> @@ -21,6 +23,8 @@
>   #include "helpers/bitmask.h"
>   
>   #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> +#define MAN_REL_PATH "/../man/man1/"
> +#define MAN_SUFFIX ".1"
>   
>   static int cmd_help(int argc, const char **argv);
>   
> @@ -80,14 +84,17 @@ static void print_help(void)
>   
>   static int print_man_page(const char *subpage)
>   {
> -	int len;
> -	char *page;
> +	char *page, *man_path, *exec_dir;
> +	char exec_path[PATH_MAX];
> +	int subpage_len;
>   
> -	len = 10; /* enough for "cpupower-" */
> -	if (subpage != NULL)
> -		len += strlen(subpage);
> +	if (!subpage)
> +		return -EINVAL;
>   
> -	page = malloc(len);
> +	subpage_len = 10; /* enough for "cpupower-" */
> +	subpage_len += strlen(subpage);
> +
> +	page = malloc(subpage_len);
>   	if (!page)
>   		return -ENOMEM;
>   
> @@ -97,7 +104,40 @@ static int print_man_page(const char *subpage)
>   		strcat(page, subpage);
>   	}
>   
> -	execlp("man", "man", page, NULL);
> +	/* Get current process image name full path */
> +	if (readlink("/proc/self/exe", exec_path, PATH_MAX) > 0) {
> +
> +		man_path = malloc(PATH_MAX);
> +		if (!man_path) {
> +			free(page);
> +			return -ENOMEM;
> +		}
> +
> +		exec_dir = strdup(exec_path);
> +		if (!exec_dir) {
> +			free(page);
> +			free(man_path);
> +			return -ENOMEM;
> +		}
> +
> +		*man_path = '\0';
> +		strncat(man_path, dirname(exec_dir), strlen(exec_dir));
> +		strncat(man_path, MAN_REL_PATH, strlen(MAN_REL_PATH));
> +		strncat(man_path, page, strlen(page));
> +		strncat(man_path, MAN_SUFFIX, strlen(MAN_SUFFIX));
> +
> +		free(exec_dir);
> +
> +		/* Check if file exists */
> +		if (access(man_path, F_OK) == -1) {
> +			free(man_path);
> +			man_path = page;
> +		}
> +	} else {
> +		man_path = page;
> +	}
> +
> +	execlp("man", "man", man_path, NULL);

You can simplify all of this by using getenv() to get the environment
variables for the program.

Take a look getenv() usages in the kernel sources for reference.
>   

thanks,
-- Shuah