tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: sysctl_doeproc() race



On 10.03.2018 21:47, Kamil Rytarowski wrote:
> On 10.03.2018 20:45, Christos Zoulas wrote:
>> In article <ef31ecad-c6ba-bc06-aace-2b7a2ff33712%gmail.com@localhost>,
>> Kamil Rytarowski  <krytarowski%gmail.com@localhost> wrote:
>>> I've been observing a race with the ptrace(2) ATF tests.
>>>
>>> There is a function await_zombie() that checks whether a process (traced
>>> by a debugger) died and is now a zombie. The current code might return a
>>> single process twice, from the allproc list and later from zombproc. I
>>> detect that in both cases the process returned twice has SDYING status.
>>>
>>> Reproducer for NetBSD, FreeBSD, Linux:
>>>
>>> http://netbsd.org/~kamil/kernel/attach2b.c
>>>
>>> This code is stable for FreeBSD and Linux, and can break on NetBSD with
>>> return value -1 and errno ENOMEM.
>>>
>>> Patch with a potential fix:
>>>
>>> http://netbsd.org/~kamil/patch-00046-zombielist-marker-sysctl_doeproc.txt
>>>
>>> I'm adding an additional marker at the head of zombproc list, and skip
>>> zombie processes that were appended to the list during the iteration
>>> over the processes.
>>>
>>> This approach generates a potential scenario when a process might be
>>> silently moved to the zombproc list. and thus becoming invisible in
>>> another race scenario.
>>>
>>> Ideally we would eliminate the proc_lock relock during the operation
>>> over all process lists. One solution would be to mark this buffer wire
>>> and disable EFAULT scenarios (and potentially disable need to relock
>>> proc_lock)... however so far nothing uses this in the kernel and I'm not
>>> sure it's proper time to go into the rabbit hole and just find a quick
>>> solution.
>>
>> Since zombie procs can't become human even if they eat brains, why don't
>> we first scan the zombie list and then the allproc list?
>>
> 
> This does not fix the race, even if we use markers at head, tail and
> every step over the lists. We will end up with the same scenario that
> allproc ones will be moving from allproc to zombieproc and we won't know
> whether they will be counted 0 times, once or twice.
> 
> There might be created another case that a process body will be reused
> for other newborn child and appended to allproc.
> 
> The only reliable way I can think of is to eliminate relocking of proc_lock.
> 

Alternatively there is a dirty solution with iterating over pid_table[],
checking P_VALID() and skipping embryonic processes, this way we will
eliminate duplicates.. however I'm not sure if this is acceptable (clean
design) solution.

>> christos
>>
> 
> 


Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index