Re: [PATCH] Added PR_SET_PROCTITLE_AREA option for prctl()

[KOSAKI Motohiro <kosaki.motohiro@gmail.com>]

Hi


>>>> -       // If the nul at the end of args has been overwritten, then
>>>> -       // assume application is using setproctitle(3).
>>>> -       if (res > 0 && buffer[res-1] != '\0' && len < PAGE_SIZE) {
>>>> +       if (mm->arg_end != mm->env_start) {
>>>> +               // PR_SET_PROCTITLE_AREA used
>>>> +               res = strnlen(buffer, res);
>>>
>>> Is this check really needed? Surely it's enough to simply state that
>>> behavior if the area isn't null-terminated is undefined.
>>
>> Well, that depends. I was hoping to use the syscall only once per process.
>> That would allow me to just update the process title whenever I feel like
>> it, possibly hundreds of times per second. This is much cheaper if I don't
>> have to use a syscall every time.
>>
>> So if I'm setting the PR_SET_PROCTITLE_AREA initially to e.g. 1 kB memory
>> area, without the above code ps will show it entirely regardless of any \0
>> characters (because parameters are separated by \0).
>
> That makes sense - but note that it's not completely atomic still -
> with a syscall you could insert some kind of barrier (rwsem?) to
> ensure other processes don't see a halfway updated process name. With
> infrequent updates this isn't a problem, but if you're really
> intending to update it at a rate where syscall overhead becomes a
> problem, then you're also going to see these kinds of issues as well.


Yes.  this patch seems buggy. The lock is necessary.
following scenario makes disaster, I think.

CPU0 (prctl caller)                                CPU1 (ps)
-------------------------------------------------------------------------------------------
mm->arg_start = arg2;
                                                                len =
mm->arg_end - mm->arg_start; in proc_pid_cmdline()
mm->arg_end = arg3;