Re: [PATCH v1 10/11] linux-user: fix /proc/self/stat handling

[Brice Goglin]

Le 10/04/2020 à 14:33, Alex Bennée a écrit :

That was by inspection on my system which seems to truncate a lot earlier. It would be nice to find where in the Linux kernel it is output but I failed to grep the relevant function last night.

It's in proc/array.c, do_task_stat() calls proc_task_name(). In the end, it seems to use task->tcomm or task->comm which is limited by

#define TASK_COMM_LEN			16

Brice

On Fri, 10 Apr 2020, 12:11 Philippe Mathieu-Daudé, <address@hidden> wrote:

Cc'ing Ludovic in case he can test with Guix-HPC.

On 4/9/20 11:15 PM, Alex Bennée wrote:
> In the original bug report long files names in Guix caused
> /proc/self/stat be truncated without the trailing ") " as specified in
> proc manpage which says:
>      (2) comm  %s
>             The  filename of the executable, in parentheses.  This
>             is visible whether or not the  executable  is  swapped
>             out.
>
> Additionally it should only be reporting the executable name rather
> than the full path. Fix both these failings while cleaning up the code
> to use GString to build up the reported values. As the whole function
> is cleaned up also adjust the white space to the current coding style.
>
> Message-ID: <address@hidden>
> Reported-by: Brice Goglin <address@hidden>
> Cc: Philippe_Mathieu-Daudé <address@hidden>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>   linux-user/syscall.c | 43 +++++++++++++++++++------------------------
>   1 file changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 6495ddc4cda..674f70e70a5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7295,34 +7295,29 @@ static int open_self_stat(void *cpu_env, int fd)
>   {
>       CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
>       TaskState *ts = cpu->opaque;
> -    abi_ulong start_stack = ts->info->start_stack;
> +    g_autoptr(GString) buf = g_string_new(NULL);
>       int i;
>   
>       for (i = 0; i < 44; i++) {
> -      char buf[128];
> -      int len;
> -      uint64_t val = 0;
> -
> -      if (i == 0) {
> -        /* pid */
> -        val = getpid();
> -        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
> -      } else if (i == 1) {
> -        /* app name */
> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> -      } else if (i == 27) {
> -        /* stack bottom */
> -        val = start_stack;
> -        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
> -      } else {
> -        /* for the rest, there is MasterCard */
> -        snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' ');
> -      }
> +        if (i == 0) {
> +            /* pid */
> +            g_string_printf(buf, FMT_pid " ", getpid());
> +        } else if (i == 1) {
> +            /* app name */
> +            gchar *bin = g_strrstr(ts->bprm->argv[0], "/");
> +            bin = bin ? bin + 1 : ts->bprm->argv[0];
> +            g_string_printf(buf, "(%.15s) ", bin);

15 or 125? 15 seems short. From your previous test I understood it was
124, for
sizeof("cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___40").

> +        } else if (i == 27) {
> +            /* stack bottom */
> +            g_string_printf(buf, TARGET_ABI_FMT_ld " ", ts->info->start_stack);
> +        } else {
> +            /* for the rest, there is MasterCard */
> +            g_string_printf(buf, "0%c", i == 43 ? '\n' : ' ');
> +        }
>   
> -      len = strlen(buf);
> -      if (write(fd, buf, len) != len) {
> -          return -1;
> -      }
> +        if (write(fd, buf->str, buf->len) != buf->len) {
> +            return -1;
> +        }
>       }
>   
>       return 0;
>