linux-kernel - Re: [PATCH 2/3] drm/scheduler: Don't call wait_event

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] [day] [month] [year] [list]

Date:   Wed, 23 May 2018 11:08:06 -0400
From:   Andrey Grodzovsky <Andrey.Grodzovsky@....com>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     christian.koenig@....com,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        David.Panariti@....com, amd-gfx@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, Alexander.Deucher@....com,
        akpm@...ux-foundation.org
Subject: Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for
 signaled process.



On 05/01/2018 10:35 AM, Oleg Nesterov wrote:
> On 04/30, Andrey Grodzovsky wrote:
>> On 04/30/2018 12:00 PM, Oleg Nesterov wrote:
>>> On 04/30, Andrey Grodzovsky wrote:
>>>> What about changing PF_SIGNALED to  PF_EXITING in
>>>> drm_sched_entity_do_release
>>>>
>>>> -       if ((current->flags & PF_SIGNALED) && current->exit_code == SIGKILL)
>>>> +      if ((current->flags & PF_EXITING) && current->exit_code == SIGKILL)
>>> let me repeat, please don't use task->exit_code. And in fact this check is racy
>>>
>>> But this doesn't matter. Say, we can trivially add SIGNAL_GROUP_KILLED_BY_SIGKILL,
>>> or do something else,
>> Can you explain where is the race and what is a possible alternative then ?
> Oh. I mentioned this race automatically, because I am pedant ;) Let me repeat
> that this doesn't really matter, and let me remind that the caller of fop->release
> can be completely unrelated process, say $cat /proc/pid/fdinfo. And in any case
> ->exit_code should not be used outside of ptrace/exit paths.
>
> OK, the race. Consider a process P with a main thread M and a sub-thread T.
>
> T does pthread_exit(), enters do_exit() and gets a preemption before exit_files().
>
> The process is killed by SIGKILL. M calls do_group_exit(), do_exit() and passes
> exit_files(). However, it doesn't call close_files() because T has another reference.
>
> T resumes, calls close_files(), fput(), etc, and then exit_task_work(), so
> it can finally call ->release() with current->exit_code == 0 desptite the fact
> the process was killed.
>
> Again, again, this doesn't matter. We can distinguish killed-or-not, by SIGKILL-
> or-not. But I still do not think we actually need this. At least in ->release()
> paths, ->flush() may differ.

Hi Oleg, reworked the code to use .flush hook and eliminated 
wait_event_killable.
So in case of .flush is this OK to look at task->exit_code == SIGKILL to 
distinguish
exit by signal ?

Andrey

>
> Oleg.
>