Christian Brauner <christian.brauner@xxxxxxxxxx> writes: > On Fri, Apr 03, 2020 at 11:11:35AM +0200, Oleg Nesterov wrote: >> On 04/02, syzbot wrote: >> > >> > lock_acquire+0x1f2/0x8f0 kernel/locking/lockdep.c:4923 >> > __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline] >> > _raw_spin_lock+0x2a/0x40 kernel/locking/spinlock.c:151 >> > spin_lock include/linux/spinlock.h:353 [inline] >> > proc_pid_make_inode+0x1f9/0x3c0 fs/proc/base.c:1880 >> >> Yes, spin_lock(wait_pidfd.lock) is not safe... >> >> Eric, at first glance the fix is simple. >> >> Oleg. >> >> >> diff --git a/fs/proc/base.c b/fs/proc/base.c > > Um, when did this lock get added to proc/base.c in the first place and > why has it been abused for this? Because struct pid is too bloated already. > People just recently complained loudly about this in the > cred_guard_mutex thread that abusing locks for things they weren't > intended for is a bad idea... The problem there is/was holding locks over places they shouldn't. It looks like I made an equally dump mistake with struct pid. That said can you take a look at calling putting do_notify_pidfd someplace sane. I can't see how it makes sense to call that in the same set of circumstances where we notify the parent. Reparenting should not be a concern, nor should ptracing. Which I think means that do_notify_pid can potentially get called many times more than it needs to be. Not to mention it is being called a bit too soon when called from do_notify_parent. Which I saw earlier is causing problems. Signal sending can call do_notify_parent early because everything just queues up and no action is taken. Wake-ups on the other hand trigger more immediate action. There is no connection to the current bug except this discussion just remimded me about do_notify_pidfd and I figured I should say something before I forget again. Eric
