All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: syzbot <syzbot+f675f964019f884dbd0f@syzkaller.appspotmail.com>,
	adobriyan@gmail.com, akpm@linux-foundation.org,
	allison@lohutok.net, areber@redhat.com,
	aubrey.li@linux.intel.com, avagin@gmail.com,
	bfields@fieldses.org, christian@brauner.io, cyphar@cyphar.com,
	ebiederm@xmission.com, gregkh@linuxfoundation.org, guro@fb.com,
	jlayton@kernel.org, joel@joelfernandes.org,
	keescook@chromium.org, linmiaohe@huawei.com,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	mhocko@suse.com, mingo@kernel.org, peterz@infradead.org,
	sargun@sargun.me, syzkaller-bugs@googlegroups.com,
	tglx@linutronix.de, viro@zeniv.linux.org.uk
Subject: Re: possible deadlock in send_sigurg
Date: Fri, 3 Apr 2020 11:36:12 +0200	[thread overview]
Message-ID: <20200403093612.mtd7edubsng24uuh@wittgenstein> (raw)
In-Reply-To: <20200403091135.GA3645@redhat.com>

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?
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...

> index 74f948a6b621..9ec8c114aa60 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1839,9 +1839,9 @@ void proc_pid_evict_inode(struct proc_inode *ei)
>  	struct pid *pid = ei->pid;
>  
>  	if (S_ISDIR(ei->vfs_inode.i_mode)) {
> -		spin_lock(&pid->wait_pidfd.lock);
> +		spin_lock_irq(&pid->wait_pidfd.lock);
>  		hlist_del_init_rcu(&ei->sibling_inodes);
> -		spin_unlock(&pid->wait_pidfd.lock);
> +		spin_unlock_irq(&pid->wait_pidfd.lock);
>  	}
>  
>  	put_pid(pid);
> @@ -1877,9 +1877,9 @@ struct inode *proc_pid_make_inode(struct super_block * sb,
>  	/* Let the pid remember us for quick removal */
>  	ei->pid = pid;
>  	if (S_ISDIR(mode)) {
> -		spin_lock(&pid->wait_pidfd.lock);
> +		spin_lock_irq(&pid->wait_pidfd.lock);
>  		hlist_add_head_rcu(&ei->sibling_inodes, &pid->inodes);
> -		spin_unlock(&pid->wait_pidfd.lock);
> +		spin_unlock_irq(&pid->wait_pidfd.lock);
>  	}
>  
>  	task_dump_owner(task, 0, &inode->i_uid, &inode->i_gid);
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index 1e730ea1dcd6..6b7ee76e1b36 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -123,9 +123,9 @@ void proc_invalidate_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock
>  		if (!node)
>  			break;
>  		ei = hlist_entry(node, struct proc_inode, sibling_inodes);
> -		spin_lock(lock);
> +		spin_lock_irq(lock);
>  		hlist_del_init_rcu(&ei->sibling_inodes);
> -		spin_unlock(lock);
> +		spin_unlock_irq(lock);
>  
>  		inode = &ei->vfs_inode;
>  		sb = inode->i_sb;
> 

  reply	other threads:[~2020-04-03  9:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-03  6:15 possible deadlock in send_sigurg syzbot
2020-04-03  9:11 ` Oleg Nesterov
2020-04-03  9:36   ` Christian Brauner [this message]
2020-04-03 12:57     ` Eric W. Biederman
2020-04-08 20:28   ` [PATCH] proc: Use a dedicated lock in struct pid Eric W. Biederman
2020-04-09  8:35     ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200403093612.mtd7edubsng24uuh@wittgenstein \
    --to=christian.brauner@ubuntu.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=allison@lohutok.net \
    --cc=areber@redhat.com \
    --cc=aubrey.li@linux.intel.com \
    --cc=avagin@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=christian@brauner.io \
    --cc=cyphar@cyphar.com \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=guro@fb.com \
    --cc=jlayton@kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=linmiaohe@huawei.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sargun@sargun.me \
    --cc=syzbot+f675f964019f884dbd0f@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.