Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > On Fri, May 8, 2020 at 11:48 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >> >> >> Oleg modified the code that did >> "mutex_lock_interruptible(¤t->cred_guard_mutex)" to return >> -ERESTARTNOINTR instead of -EINTR, so that userspace will never see a >> failure to grab the mutex. >> >> Slightly earlier Liam R. Howlett defined mutex_lock_killable for >> exactly the same situation but it does it a little more cleanly. > > What what what? > > None of this makes sense. Your commit message is completely wrong, and > the patch is utter shite. > > mutex_lock_interruptible() and mutex_lock_killable() are completely > different operations, and the difference has absolutely nothing to do > with -ERESTARTNOINTR or -EINTR. > > mutex_lock_interruptible() is interrupted by any signal. > > mutex_lock_killable() is - surprise surprise - only interrupted by > SIGKILL (in theory any fatal signal, but we never actually implemented > that logic, so it's only interruptible by the known-to-always-be-fatal > SIGKILL). > >> Switch the code to mutex_lock_killable so that it is clearer what the >> code is doing. > > This nonsensical patch makes me worry about all your other patches. > The explanation is wrong, the patch is wrong, and it changes things to > be fundamentally broken. > > Before this, ^C would break out of a blocked execve()/ptrace() > situation. After this patch, you need special tools to do so. > > This patch is completely wrong. Sigh. Brain fart on my part. You are correct. I saw the restart, and totally forgot that it allows the handling of a signal before restarting the system call. Except for the handling of the signal in userspace it is the same as mutex_lock_killable but that is a big big big if. My apologies. I will drop this patch. Eric
