linux-user: keep the name-ending parenthesis in /proc/self/stat

linux-user: keep the name-ending parenthesis in /proc/self/stat

[Brice Goglin]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

When the program name is very long, qemu-user may truncate it in
/proc/self/stat. However the truncation must keep the ending ") "
to conform to the 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.

To reproduce:
$ ln -s /bin/cat <filenamewithmorethan128chars>
$ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat

Before the patch, you get:
1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
After the patch:
1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...

This fixes an issue with hwloc failing to parse /proc/self/stat
when Ludovic Courtes was testing it in guix over qemu-aarch64.

Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5af55fca78..a1126dcf5b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
         snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
       } else if (i == 1) {
         /* app name */
-        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+        len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+        if (len >= sizeof(buf))
+          /* bring back the ending ") " that was truncated */
+          strcpy(buf+sizeof(buf)-3, ") ");
       } else if (i == 27) {
         /* stack bottom */
         val = start_stack;


[-- Attachment #2: Type: text/html, Size: 1738 bytes --]

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Philippe Mathieu-Daudé]

On 3/30/20 9:07 PM, Brice Goglin wrote:
> When the program name is very long, qemu-user may truncate it in
> /proc/self/stat. However the truncation must keep the ending ") "
> to conform to the 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.
> 
> To reproduce:
> $ ln -s /bin/cat <filenamewithmorethan128chars>
> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
> 
> Before the patch, you get:
> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
> After the patch:
> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
> 
> This fixes an issue with hwloc failing to parse /proc/self/stat
> when Ludovic Courtes was testing it in guix over qemu-aarch64.
> 
> Signed-off-by: Brice Goglin<Brice.Goglin@inria.fr>
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5af55fca78..a1126dcf5b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
>           snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>         } else if (i == 1) {
>           /* app name */
> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> +        len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> +        if (len >= sizeof(buf))
> +          /* bring back the ending ") " that was truncated */
> +          strcpy(buf+sizeof(buf)-3, ") ");

Maybe we can avoid the sprintf() call:

-- >8 --
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
        } else if (i == 1) {
          /* app name */
-        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+        char *ptr = buf;
+
+        *ptr++ = '(';
+        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
+        strcpy(ptr, ") ");
        } else if (i == 27) {
          /* stack bottom */
          val = start_stack;
---

>         } else if (i == 27) {
>           /* stack bottom */
>           val = start_stack;
> 

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Brice Goglin]

Le 31/03/2020 à 00:05, Philippe Mathieu-Daudé a écrit :
> On 3/30/20 9:07 PM, Brice Goglin wrote:
>> When the program name is very long, qemu-user may truncate it in
>> /proc/self/stat. However the truncation must keep the ending ") "
>> to conform to the 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.
>>
>> To reproduce:
>> $ ln -s /bin/cat <filenamewithmorethan128chars>
>> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>>
>> Before the patch, you get:
>> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
>> After the patch:
>> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>>
>> This fixes an issue with hwloc failing to parse /proc/self/stat
>> when Ludovic Courtes was testing it in guix over qemu-aarch64.
>>
>> Signed-off-by: Brice Goglin<Brice.Goglin@inria.fr>
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 5af55fca78..a1126dcf5b 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
>>           snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>         } else if (i == 1) {
>>           /* app name */
>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +        len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +        if (len >= sizeof(buf))
>> +          /* bring back the ending ") " that was truncated */
>> +          strcpy(buf+sizeof(buf)-3, ") ");
>
> Maybe we can avoid the sprintf() call:
>
> -- >8 --
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>        } else if (i == 1) {
>          /* app name */
> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> +        char *ptr = buf;
> +
> +        *ptr++ = '(';
> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
> +        strcpy(ptr, ") ");
>        } else if (i == 27) {
>          /* stack bottom */
>          val = start_stack;
>

This works too.

Brice

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Brice Goglin]

Le 31/03/2020 à 00:29, Brice Goglin a écrit :
> Le 31/03/2020 à 00:05, Philippe Mathieu-Daudé a écrit :
>> On 3/30/20 9:07 PM, Brice Goglin wrote:
>>> When the program name is very long, qemu-user may truncate it in
>>> /proc/self/stat. However the truncation must keep the ending ") "
>>> to conform to the 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.
>>>
>>> To reproduce:
>>> $ ln -s /bin/cat <filenamewithmorethan128chars>
>>> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>>>
>>> Before the patch, you get:
>>> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
>>> After the patch:
>>> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>>>
>>> This fixes an issue with hwloc failing to parse /proc/self/stat
>>> when Ludovic Courtes was testing it in guix over qemu-aarch64.
>>>
>>> Signed-off-by: Brice Goglin<Brice.Goglin@inria.fr>
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 5af55fca78..a1126dcf5b 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
>>>           snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>>         } else if (i == 1) {
>>>           /* app name */
>>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +        len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +        if (len >= sizeof(buf))
>>> +          /* bring back the ending ") " that was truncated */
>>> +          strcpy(buf+sizeof(buf)-3, ") ");
>> Maybe we can avoid the sprintf() call:
>>
>> -- >8 --
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>        } else if (i == 1) {
>>          /* app name */
>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +        char *ptr = buf;
>> +
>> +        *ptr++ = '(';
>> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>> +        strcpy(ptr, ") ");
>>        } else if (i == 27) {
>>          /* stack bottom */
>>          val = start_stack;
>>
> This works too.


Hello

Is anybody going to fix this anyhow for the next release?

Thank you

Brice

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Laurent Vivier]

Le 08/04/2020 à 08:40, Brice Goglin a écrit :
> Le 31/03/2020 à 00:29, Brice Goglin a écrit :
>> Le 31/03/2020 à 00:05, Philippe Mathieu-Daudé a écrit :
>>> On 3/30/20 9:07 PM, Brice Goglin wrote:
>>>> When the program name is very long, qemu-user may truncate it in
>>>> /proc/self/stat. However the truncation must keep the ending ") "
>>>> to conform to the 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.
>>>>
>>>> To reproduce:
>>>> $ ln -s /bin/cat <filenamewithmorethan128chars>
>>>> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>>>>
>>>> Before the patch, you get:
>>>> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
>>>> After the patch:
>>>> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>>>>
>>>> This fixes an issue with hwloc failing to parse /proc/self/stat
>>>> when Ludovic Courtes was testing it in guix over qemu-aarch64.
>>>>
>>>> Signed-off-by: Brice Goglin<Brice.Goglin@inria.fr>
>>>>
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index 5af55fca78..a1126dcf5b 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -7305,7 +7305,10 @@ static int open_self_stat(void *cpu_env, int fd)
>>>>           snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>>>         } else if (i == 1) {
>>>>           /* app name */
>>>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>>> +        len = snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>>> +        if (len >= sizeof(buf))
>>>> +          /* bring back the ending ") " that was truncated */
>>>> +          strcpy(buf+sizeof(buf)-3, ") ");
>>> Maybe we can avoid the sprintf() call:
>>>
>>> -- >8 --
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>>        } else if (i == 1) {
>>>          /* app name */
>>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +        char *ptr = buf;
>>> +
>>> +        *ptr++ = '(';
>>> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>>> +        strcpy(ptr, ") ");
>>>        } else if (i == 27) {
>>>          /* stack bottom */
>>>          val = start_stack;
>>>
>> This works too.
> 
> 
> Hello
> 
> Is anybody going to fix this anyhow for the next release?

Hi,

we are expecting you fix the patch according to Philippe's comment and
re-send it or explain why you don't want.

Thanks,
Laurent

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Alex Bennée]

Brice Goglin <Brice.Goglin@inria.fr> writes:

> Le 09/04/2020 à 17:27, Alex Bennée a écrit :
>> Brice Goglin <Brice.Goglin@inria.fr> writes:
>>
>>> When the program name is very long, qemu-user may truncate it in
>>> /proc/self/stat. However the truncation must keep the ending ") "
>>> to conform to the 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.

Huh testing on my box here it seems to truncate a lot earlier than that:

20:54:41 [alex@zen:~/l/q/b/all] sanitiser/fixes-for-5.1|●1✚1…(+1/-1) +
./cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___4567890 /proc/self/stat
23132 (cat_with9_12345) R 15690 23132 15676 34827 23132 4194304 87 0 0 0 0 0 0 0 20 0 1 0 133272440 6172672 188 18446744073709551615 94698916007936 94698916032905 140729243846896 0 0 0 0 0 0 0 0 0 17 2 0 0 0 0 0 94698916052048 94698916053600 94698933542912 140729243849857 140729243850006 140729243850006 140729243852659 0

20:55:21 [alex@zen:~/l/q/b/all] sanitiser/fixes-for-5.1|●1✚1…(+1/-1) 126 +
./x86_64-linux-user/qemu-x86_64  ./cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___4567890 /proc/s
elf/stat
23519 (./cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___40 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 274903122400 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

>>>
>>> To reproduce:
>>> $ ln -s /bin/cat <filenamewithmorethan128chars>
>>> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>>>
>>> Before the patch, you get:
>>> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
>>> After the patch:
>>> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>>>
>>> This fixes an issue with hwloc failing to parse /proc/self/stat
>>> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>>>
>>> Signed-off-by: Philippe_Mathieu-Daudé <philmd@redhat.com>
>>> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>>        } else if (i == 1) {
>>>          /* app name */
>>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +        char *ptr = buf;
>>> +
>>> +        *ptr++ = '(';
>>> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>>> +        strcpy(ptr, ") ");
>> why not just use a format string:
>>
>>   snprintf(buf, sizeof(buf), "(%.125s) ", ts->bprm->argv[0]);
>>
>
> Go ahead and apply what you want (maybe 124 instead of 125 because of
> the ending \0).
>
> My commit message above explains how to test things very quickly.
>
> I don't use qemu-user or Guix myself, and I can't spend time
> debugging/testing this further.
>
> Thank you
>
> Brice


-- 
Alex Bennée

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Brice Goglin]

Le 09/04/2020 à 17:27, Alex Bennée a écrit :
> Brice Goglin <Brice.Goglin@inria.fr> writes:
>
>> When the program name is very long, qemu-user may truncate it in
>> /proc/self/stat. However the truncation must keep the ending ") "
>> to conform to the 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.
>>
>> To reproduce:
>> $ ln -s /bin/cat <filenamewithmorethan128chars>
>> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>>
>> Before the patch, you get:
>> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
>> After the patch:
>> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>>
>> This fixes an issue with hwloc failing to parse /proc/self/stat
>> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>>
>> Signed-off-by: Philippe_Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>        } else if (i == 1) {
>>          /* app name */
>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +        char *ptr = buf;
>> +
>> +        *ptr++ = '(';
>> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>> +        strcpy(ptr, ") ");
> why not just use a format string:
>
>   snprintf(buf, sizeof(buf), "(%.125s) ", ts->bprm->argv[0]);
>

Go ahead and apply what you want (maybe 124 instead of 125 because of
the ending \0).

My commit message above explains how to test things very quickly.

I don't use qemu-user or Guix myself, and I can't spend time
debugging/testing this further.

Thank you

Brice

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Alex Bennée]

Brice Goglin <Brice.Goglin@inria.fr> writes:

> When the program name is very long, qemu-user may truncate it in
> /proc/self/stat. However the truncation must keep the ending ") "
> to conform to the 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.
>
> To reproduce:
> $ ln -s /bin/cat <filenamewithmorethan128chars>
> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>
> Before the patch, you get:
> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
> After the patch:
> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>
> This fixes an issue with hwloc failing to parse /proc/self/stat
> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>
> Signed-off-by: Philippe_Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>        } else if (i == 1) {
>          /* app name */
> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> +        char *ptr = buf;
> +
> +        *ptr++ = '(';
> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
> +        strcpy(ptr, ") ");

why not just use a format string:

  snprintf(buf, sizeof(buf), "(%.125s) ", ts->bprm->argv[0]);

although to be honest when ever I see a bunch of sizeof(buf) code for
strings I tend to re-write it to use the glib GString functions.

>        } else if (i == 27) {
>          /* stack bottom */
>          val = start_stack;


-- 
Alex Bennée

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Brice Goglin]

Please apply my first patch if you believe Philippe's patch is wrong. I
can't spend more time debugging this trivial issue unfortunately.

Brice



Le 08/04/2020 à 17:48, Laurent Vivier a écrit :
> Le 08/04/2020 à 10:24, Brice Goglin a écrit :
>> When the program name is very long, qemu-user may truncate it in
>> /proc/self/stat. However the truncation must keep the ending ") "
>> to conform to the 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.
>>
>> To reproduce:
>> $ ln -s /bin/cat <filenamewithmorethan128chars>
>> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
>>
>> Before the patch, you get:
>> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
>> After the patch:
>> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
>>
>> This fixes an issue with hwloc failing to parse /proc/self/stat
>> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>>
>> Signed-off-by: Philippe_Mathieu-Daudé <philmd@redhat.com>
> You can't add "Signed-off-by" of someone else, in this case you could
> add "Suggested-by:".
>
> The subject of your patch should include "[PATCH]" (and the version of
> the patch, "[PATCH v2]").
>
> https://wiki.qemu.org/Contribute/SubmitAPatch
>
>> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>        } else if (i == 1) {
>>          /* app name */
>> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> +        char *ptr = buf;
>> +
>> +        *ptr++ = '(';
>> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
> To have space for the NUL character I think it should be "sizeof(buf) - 4".
>
>> +        strcpy(ptr, ") ");
>>        } else if (i == 27) {
>>          /* stack bottom */
>>          val = start_stack;
>>
> Thanks,
> Laurent

Re: linux-user: keep the name-ending parenthesis in /proc/self/stat

[Laurent Vivier]

Le 08/04/2020 à 10:24, Brice Goglin a écrit :
> When the program name is very long, qemu-user may truncate it in
> /proc/self/stat. However the truncation must keep the ending ") "
> to conform to the 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.
> 
> To reproduce:
> $ ln -s /bin/cat <filenamewithmorethan128chars>
> $ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat
> 
> Before the patch, you get:
> 1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
> After the patch:
> 1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...
> 
> This fixes an issue with hwloc failing to parse /proc/self/stat
> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
> 
> Signed-off-by: Philippe_Mathieu-Daudé <philmd@redhat.com>

You can't add "Signed-off-by" of someone else, in this case you could
add "Suggested-by:".

The subject of your patch should include "[PATCH]" (and the version of
the patch, "[PATCH v2]").

https://wiki.qemu.org/Contribute/SubmitAPatch

> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>          snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>        } else if (i == 1) {
>          /* app name */
> -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
> +        char *ptr = buf;
> +
> +        *ptr++ = '(';
> +        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);

To have space for the NUL character I think it should be "sizeof(buf) - 4".

> +        strcpy(ptr, ") ");
>        } else if (i == 27) {
>          /* stack bottom */
>          val = start_stack;
> 

Thanks,
Laurent

linux-user: keep the name-ending parenthesis in /proc/self/stat

[Brice Goglin]

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

When the program name is very long, qemu-user may truncate it in
/proc/self/stat. However the truncation must keep the ending ") "
to conform to the 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.

To reproduce:
$ ln -s /bin/cat <filenamewithmorethan128chars>
$ qemu-x86_64 ./<filenamewithmorethan128chars> /proc/self/stat

Before the patch, you get:
1134631 (<filenametruncated>0 0 0 0 0 0 0 0 ...
After the patch:
1134631 (<filenametruncat>) 0 0 0 0 0 0 0 0 ...

This fixes an issue with hwloc failing to parse /proc/self/stat
when Ludovic Courtes was testing it in Guix over qemu-aarch64.

Signed-off-by: Philippe_Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
         snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
       } else if (i == 1) {
         /* app name */
-        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
+        char *ptr = buf;
+
+        *ptr++ = '(';
+        ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
+        strcpy(ptr, ") ");
       } else if (i == 27) {
         /* stack bottom */
         val = start_stack;


[-- Attachment #2: Type: text/html, Size: 1886 bytes --]