All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	syzbot+22e87cdf94021b984aa6@syzkaller.appspotmail.com,
	syzbot+c5e32344981ad9f33750@syzkaller.appspotmail.com,
	Boqun Feng <boqun.feng@gmail.com>,
	Jeff Layton <jlayton@kernel.org>
Subject: [PATCH 5.4 33/47] fcntl: Fix potential deadlock in send_sig{io, urg}()
Date: Mon,  4 Jan 2021 16:57:32 +0100	[thread overview]
Message-ID: <20210104155707.335126873@linuxfoundation.org> (raw)
In-Reply-To: <20210104155705.740576914@linuxfoundation.org>

From: Boqun Feng <boqun.feng@gmail.com>

commit 8d1ddb5e79374fb277985a6b3faa2ed8631c5b4c upstream.

Syzbot reports a potential deadlock found by the newly added recursive
read deadlock detection in lockdep:

[...] ========================================================
[...] WARNING: possible irq lock inversion dependency detected
[...] 5.9.0-rc2-syzkaller #0 Not tainted
[...] --------------------------------------------------------
[...] syz-executor.1/10214 just changed the state of lock:
[...] ffff88811f506338 (&f->f_owner.lock){.+..}-{2:2}, at: send_sigurg+0x1d/0x200
[...] but this lock was taken by another, HARDIRQ-safe lock in the past:
[...]  (&dev->event_lock){-...}-{2:2}
[...]
[...]
[...] and interrupts could create inverse lock ordering between them.
[...]
[...]
[...] other info that might help us debug this:
[...] Chain exists of:
[...]   &dev->event_lock --> &new->fa_lock --> &f->f_owner.lock
[...]
[...]  Possible interrupt unsafe locking scenario:
[...]
[...]        CPU0                    CPU1
[...]        ----                    ----
[...]   lock(&f->f_owner.lock);
[...]                                local_irq_disable();
[...]                                lock(&dev->event_lock);
[...]                                lock(&new->fa_lock);
[...]   <Interrupt>
[...]     lock(&dev->event_lock);
[...]
[...]  *** DEADLOCK ***

The corresponding deadlock case is as followed:

	CPU 0		CPU 1		CPU 2
	read_lock(&fown->lock);
			spin_lock_irqsave(&dev->event_lock, ...)
					write_lock_irq(&filp->f_owner.lock); // wait for the lock
			read_lock(&fown-lock); // have to wait until the writer release
					       // due to the fairness
	<interrupted>
	spin_lock_irqsave(&dev->event_lock); // wait for the lock

The lock dependency on CPU 1 happens if there exists a call sequence:

	input_inject_event():
	  spin_lock_irqsave(&dev->event_lock,...);
	  input_handle_event():
	    input_pass_values():
	      input_to_handler():
	        handler->event(): // evdev_event()
	          evdev_pass_values():
	            spin_lock(&client->buffer_lock);
	            __pass_event():
	              kill_fasync():
	                kill_fasync_rcu():
	                  read_lock(&fa->fa_lock);
	                  send_sigio():
	                    read_lock(&fown->lock);

To fix this, make the reader in send_sigurg() and send_sigio() use
read_lock_irqsave() and read_lock_irqrestore().

Reported-by: syzbot+22e87cdf94021b984aa6@syzkaller.appspotmail.com
Reported-by: syzbot+c5e32344981ad9f33750@syzkaller.appspotmail.com
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/fcntl.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -779,9 +779,10 @@ void send_sigio(struct fown_struct *fown
 {
 	struct task_struct *p;
 	enum pid_type type;
+	unsigned long flags;
 	struct pid *pid;
 	
-	read_lock(&fown->lock);
+	read_lock_irqsave(&fown->lock, flags);
 
 	type = fown->pid_type;
 	pid = fown->pid;
@@ -802,7 +803,7 @@ void send_sigio(struct fown_struct *fown
 		read_unlock(&tasklist_lock);
 	}
  out_unlock_fown:
-	read_unlock(&fown->lock);
+	read_unlock_irqrestore(&fown->lock, flags);
 }
 
 static void send_sigurg_to_task(struct task_struct *p,
@@ -817,9 +818,10 @@ int send_sigurg(struct fown_struct *fown
 	struct task_struct *p;
 	enum pid_type type;
 	struct pid *pid;
+	unsigned long flags;
 	int ret = 0;
 	
-	read_lock(&fown->lock);
+	read_lock_irqsave(&fown->lock, flags);
 
 	type = fown->pid_type;
 	pid = fown->pid;
@@ -842,7 +844,7 @@ int send_sigurg(struct fown_struct *fown
 		read_unlock(&tasklist_lock);
 	}
  out_unlock_fown:
-	read_unlock(&fown->lock);
+	read_unlock_irqrestore(&fown->lock, flags);
 	return ret;
 }
 



  parent reply	other threads:[~2021-01-04 16:00 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-04 15:56 [PATCH 5.4 00/47] 5.4.87-rc1 review Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 01/47] net/sched: sch_taprio: reset child qdiscs before freeing them Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 02/47] md/raid10: initialize r10_bio->read_slot before use Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 03/47] thermal/drivers/cpufreq_cooling: Update cpufreq_state only if state has changed Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 04/47] ext4: prevent creating duplicate encrypted filenames Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 05/47] ubifs: " Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 06/47] f2fs: " Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 07/47] fscrypt: add fscrypt_is_nokey_name() Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 08/47] fscrypt: remove kernel-internal constants from UAPI header Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 09/47] vfio/pci: Move dummy_resources_list init in vfio_pci_probe() Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 10/47] btrfs: fix race when defragmenting leads to unnecessary IO Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 11/47] ext4: dont remount read-only with errors=continue on reboot Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 12/47] KVM: x86: avoid incorrect writes to host MSR_IA32_SPEC_CTRL Greg Kroah-Hartman
2021-02-26 11:03   ` Thomas Lamprecht
2021-02-26 11:27     ` Paolo Bonzini
2021-02-26 12:59       ` Greg Kroah-Hartman
2021-02-26 14:15         ` Paolo Bonzini
2021-02-26 14:18           ` Thomas Lamprecht
2021-02-26 14:21             ` Paolo Bonzini
2021-01-04 15:57 ` [PATCH 5.4 13/47] KVM: SVM: relax conditions for allowing MSR_IA32_SPEC_CTRL accesses Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 14/47] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 15/47] powerpc/bitops: Fix possible undefined behaviour with fls() and fls64() Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 16/47] jffs2: Allow setting rp_size to zero during remounting Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 17/47] jffs2: Fix NULL pointer dereference in rp_size fs option parsing Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 18/47] scsi: block: Fix a race in the runtime power management code Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 19/47] uapi: move constants from <linux/kernel.h> to <linux/const.h> Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 20/47] tools headers UAPI: Sync linux/const.h with the kernel headers Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 21/47] null_blk: Fix zone size initialization Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 22/47] of: fix linker-section match-table corruption Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 23/47] cgroup: Fix memory leak when parsing multiple source parameters Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 24/47] scsi: cxgb4i: Fix TLS dependency Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 25/47] Bluetooth: hci_h5: close serdev device and free hu in h5_close Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 26/47] reiserfs: add check for an invalid ih_entry_count Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 27/47] misc: vmw_vmci: fix kernel info-leak by initializing dbells in vmci_ctx_get_chkpt_doorbells() Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 28/47] media: gp8psk: initialize stats at power control logic Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 29/47] f2fs: fix shift-out-of-bounds in sanity_check_raw_super() Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 30/47] ALSA: seq: Use bool for snd_seq_queue internal flags Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 31/47] ALSA: rawmidi: Access runtime->avail always in spinlock Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 32/47] bfs: dont use WARNING: string when its just info Greg Kroah-Hartman
2021-01-04 15:57 ` Greg Kroah-Hartman [this message]
2021-01-04 15:57 ` [PATCH 5.4 34/47] rtc: sun6i: Fix memleak in sun6i_rtc_clk_init Greg Kroah-Hartman
2021-01-06 13:07   ` Pavel Machek
2021-01-04 15:57 ` [PATCH 5.4 35/47] module: set MODULE_STATE_GOING state when a module fails to load Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 36/47] quota: Dont overflow quota file offsets Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 37/47] rtc: pl031: fix resource leak in pl031_probe Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 38/47] powerpc: sysdev: add missing iounmap() on error in mpic_msgr_probe() Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 39/47] i3c master: fix missing destroy_workqueue() on error in i3c_master_register Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 40/47] NFSv4: Fix a pNFS layout related use-after-free race when freeing the inode Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 41/47] f2fs: avoid race condition for shrinker count Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 42/47] module: delay kobject uevent until after module init call Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 43/47] fs/namespace.c: WARN if mnt_count has become negative Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 44/47] um: ubd: Submit all data segments atomically Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 45/47] tick/sched: Remove bogus boot "safety" check Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 46/47] ALSA: pcm: Clear the full allocated memory at hw_params Greg Kroah-Hartman
2021-01-04 15:57 ` [PATCH 5.4 47/47] dm verity: skip verity work if I/O error when system is shutting down Greg Kroah-Hartman
2021-01-04 21:09 ` [PATCH 5.4 00/47] 5.4.87-rc1 review Jon Hunter
2021-01-05  6:07 ` Daniel Díaz
2021-01-05 16:39 ` Shuah Khan
2021-01-05 18:17 ` Guenter Roeck

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=20210104155707.335126873@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+22e87cdf94021b984aa6@syzkaller.appspotmail.com \
    --cc=syzbot+c5e32344981ad9f33750@syzkaller.appspotmail.com \
    /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.