On 4/30/20 6:40 PM, Linus Torvalds wrote: > On Thu, Apr 30, 2020 at 7:29 AM Bernd Edlinger > <bernd.edlin...@hotmail.de> wrote: >> >> Ah, now I see, that was of course not the intended effect, >> but that is not where the pseudo-deadlock happens at all, >> would returning -RESTARTNOINTR in this function make this >> patch acceptable, it will not have an effect on the test case? > > So that was why I suggested doing it all with a helper function, and > also doing that > > set_thread_flag(TIF_SIGPENDING); > > because without going through the "check-for-signals" code at return > to user space, -ERESTARTNOINTR doesn't actually _do_ any restart. > > However, the more I looked at it, the less I actually liked that hack. > > Part of it is simply because it can cause the exact same problem that > ptrace() does (at least in theory). And even if you don't get the > livelock thing, you can get the "use 100% CPU time" thing, because if > that case ever triggers, and we re-try, it will generally just _keep_ > on triggering (think "execve is waiting for a zombie, nobody is > reaping it"). > > IOW, restarting doesn't really fix the problem, or guarantee any > forward progress. >
Right, if it is a real time process it will result in priority-inversion. Correct. If it is a virus checker it will be real time priority and it will be very aggressive ;-) I can feel its aggressiveness already :-) shiver... And this little zombie-process will paralyze it immediately, nice try. You see what I mean? > So I'd have been ok with your "unsafe_exec_flag" if > > (a) it had been done in one place with a helper function. > > (b) it would _only_ trigger for ptrace (and perhaps seccomp). > > but I don't think it works for that write() case. > > That said, I'm not 100% convinced that that write() case really even > needs that cred_guard_mutex (renamed or not). > > Maybe we can introduce a new mutex just against concurrent ptrace > (which is what at least the _comment_ says_ that > security_setprocattr() wants - I didn't check the actual low-level > security code). > > So maybe that proc_pid_attr_write() case could be done some other way > entirely. > > Th emore we go through all this, the more I really think that Oleg's > patch to just delay the waiting for things until after dropping the > mutex in execve() is the way to go. > > Is it a "simple" and small patch? No. But it really addresses the core > issue, without introducing new odd rules or special cases, or making a > lock that doesn't reliably work as a lock. > Hmm. I think I can agree, that this problem deserves to be solved really slowly. Oleg where was your last patch, does it still work or does it need to be re-based? And I almost forgot about Eric, are you still with us? Thanks Bernd.