public inbox for linux-kernel@vger.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,
	Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>,
	Amit Shah <amit.shah@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Rusty Russell <rusty@rustcorp.com.au>
Subject: [ 33/60] virtio/console: Add pipe_lock/unlock for splice_write
Date: Mon, 12 Aug 2013 23:34:00 -0700	[thread overview]
Message-ID: <20130813063336.785700982@linuxfoundation.org> (raw)
In-Reply-To: <20130813063331.950321483@linuxfoundation.org>

3.10-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>

commit 2b4fbf029dff5a28d9bf646346dea891ec43398a upstream.

Add pipe_lock/unlock for splice_write to avoid oops by following competition:

(1) An application gets fds of a trace buffer, virtio-serial, pipe.
(2) The application does fork()
(3) The processes execute splice_read(trace buffer) and
    splice_write(virtio-serial) via same pipe.

        <parent>                   <child>
  get fds of a trace buffer,
         virtio-serial, pipe
          |
        fork()----------create--------+
          |                           |
      splice(read)                    |           ---+
      splice(write)                   |              +-- no competition
          |                       splice(read)       |
          |                       splice(write)   ---+
          |                           |
      splice(read)                    |
      splice(write)               splice(read)    ------ competition
          |                       splice(write)

Two processes share a pipe_inode_info structure. If the child execute
splice(read) when the parent tries to execute splice(write), the
structure can be broken. Existing virtio-serial driver does not get
lock for the structure in splice_write, so this competition will induce
oops.

<oops messages>
 BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
 IP: [<ffffffff811a6b5f>] splice_from_pipe_feed+0x6f/0x130
 PGD 7223e067 PUD 72391067 PMD 0
 Oops: 0000 [#1] SMP
 Modules linked in: lockd bnep bluetooth rfkill sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_page_alloc snd_timer snd soundcore pcspkr virtio_net virtio_balloon i2c_piix4 i2c_core microcode uinput floppy
 CPU: 0 PID: 1072 Comm: compete-test Not tainted 3.10.0ws+ #55
 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
 task: ffff880071b98000 ti: ffff88007b55e000 task.ti: ffff88007b55e000
 RIP: 0010:[<ffffffff811a6b5f>]  [<ffffffff811a6b5f>] splice_from_pipe_feed+0x6f/0x130
 RSP: 0018:ffff88007b55fd78  EFLAGS: 00010287
 RAX: 0000000000000000 RBX: ffff88007b55fe20 RCX: 0000000000000000
 RDX: 0000000000001000 RSI: ffff88007a95ba30 RDI: ffff880036f9e6c0
 RBP: ffff88007b55fda8 R08: 00000000000006ec R09: ffff880077626708
 R10: 0000000000000003 R11: ffffffff8139ca59 R12: ffff88007a95ba30
 R13: 0000000000000000 R14: ffffffff8139dd00 R15: ffff880036f9e6c0
 FS:  00007f2e2e3a0740(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
 CR2: 0000000000000018 CR3: 0000000071bd1000 CR4: 00000000000006f0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
 Stack:
  ffffffff8139ca59 ffff88007b55fe20 ffff880036f9e6c0 ffffffff8139dd00
  ffff8800776266c0 ffff880077626708 ffff88007b55fde8 ffffffff811a6e8e
  ffff88007b55fde8 ffffffff8139ca59 ffff880036f9e6c0 ffff88007b55fe20
 Call Trace:
  [<ffffffff8139ca59>] ? alloc_buf.isra.13+0x39/0xb0
  [<ffffffff8139dd00>] ? virtcons_restore+0x100/0x100
  [<ffffffff811a6e8e>] __splice_from_pipe+0x7e/0x90
  [<ffffffff8139ca59>] ? alloc_buf.isra.13+0x39/0xb0
  [<ffffffff8139d739>] port_fops_splice_write+0xe9/0x140
  [<ffffffff8127a3f4>] ? selinux_file_permission+0xc4/0x120
  [<ffffffff8139d650>] ? wait_port_writable+0x1b0/0x1b0
  [<ffffffff811a6fe0>] do_splice_from+0xa0/0x110
  [<ffffffff811a951f>] SyS_splice+0x5ff/0x6b0
  [<ffffffff8161facf>] tracesys+0xdd/0xe2
 Code: 49 8b 87 80 00 00 00 4c 8d 24 d0 8b 53 04 41 8b 44 24 0c 4d 8b 6c 24 10 39 d0 89 03 76 02 89 13 49 8b 44 24 10 4c 89 e6 4c 89 ff <ff> 50 18 85 c0 0f 85 aa 00 00 00 48 89 da 4c 89 e6 4c 89 ff 41
 RIP  [<ffffffff811a6b5f>] splice_from_pipe_feed+0x6f/0x130
  RSP <ffff88007b55fd78>
 CR2: 0000000000000018
 ---[ end trace 24572beb7764de59 ]---

V2: Fix a locking problem for error
V3: Add Reviewed-by lines and stable@ line in sign-off area

Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Amit Shah <amit.shah@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/char/virtio_console.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -936,16 +936,21 @@ static ssize_t port_fops_splice_write(st
 	 * pipe->nrbufs == 0 means there are no data to transfer,
 	 * so this returns just 0 for no data.
 	 */
-	if (!pipe->nrbufs)
-		return 0;
+	pipe_lock(pipe);
+	if (!pipe->nrbufs) {
+		ret = 0;
+		goto error_out;
+	}
 
 	ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
 	if (ret < 0)
-		return ret;
+		goto error_out;
 
 	buf = alloc_buf(port->out_vq, 0, pipe->nrbufs);
-	if (!buf)
-		return -ENOMEM;
+	if (!buf) {
+		ret = -ENOMEM;
+		goto error_out;
+	}
 
 	sgl.n = 0;
 	sgl.len = 0;
@@ -953,12 +958,17 @@ static ssize_t port_fops_splice_write(st
 	sgl.sg = buf->sg;
 	sg_init_table(sgl.sg, sgl.size);
 	ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
+	pipe_unlock(pipe);
 	if (likely(ret > 0))
 		ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true);
 
 	if (unlikely(ret <= 0))
 		free_buf(buf, true);
 	return ret;
+
+error_out:
+	pipe_unlock(pipe);
+	return ret;
 }
 
 static unsigned int port_fops_poll(struct file *filp, poll_table *wait)



  parent reply	other threads:[~2013-08-13  6:33 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-13  6:33 [ 00/60] 3.10.7-stable review Greg Kroah-Hartman
2013-08-13  6:33 ` [ 01/60] SCSI: Dont attempt to send extended INQUIRY command if skip_vpd_pages is set Greg Kroah-Hartman
2013-08-13  6:33 ` [ 02/60] SCSI: megaraid_sas: megaraid_sas driver init fails in kdump kernel Greg Kroah-Hartman
2013-08-13  6:33 ` [ 03/60] virtio-scsi: Fix virtqueue affinity setup Greg Kroah-Hartman
2013-08-13  6:33 ` [ 04/60] powerpc: On POWERNV enable PPC_DENORMALISATION by default Greg Kroah-Hartman
2013-08-13  6:33 ` [ 05/60] powerpc: Fix hypervisor facility unavaliable vector number Greg Kroah-Hartman
2013-08-13  6:33 ` [ 06/60] powerpc: Rework setting up H/FSCR bit definitions Greg Kroah-Hartman
2013-08-13  6:33 ` [ 07/60] powerpc: Fix context switch DSCR on POWER8 Greg Kroah-Hartman
2013-08-13  6:33 ` [ 08/60] powerpc: Save the TAR register earlier Greg Kroah-Hartman
2013-08-13  6:33 ` [ 09/60] powerpc/tm: Fix context switching TAR, PPR and DSCR SPRs Greg Kroah-Hartman
2013-08-13  6:33 ` [ 10/60] ext4: destroy ext4_es_cachep on module unload Greg Kroah-Hartman
2013-08-13  6:33 ` [ 11/60] ext4: make sure group number is bumped after a inode allocation race Greg Kroah-Hartman
2013-08-13  6:33 ` [ 12/60] ext4: fix retry handling in ext4_ext_truncate() Greg Kroah-Hartman
2013-08-13  6:33 ` [ 13/60] regmap: cache: Make sure to sync the last register in a block Greg Kroah-Hartman
2013-08-13  6:33 ` [ 14/60] regmap: Add missing header for !CONFIG_REGMAP stubs Greg Kroah-Hartman
2013-08-13  6:33 ` [ 15/60] hwmon: (adt7470) Fix incorrect return code check Greg Kroah-Hartman
2013-08-13  6:33 ` [ 16/60] staging: zcache: fix "zcache=" kernel parameter Greg Kroah-Hartman
2013-08-13  6:33 ` [ 17/60] media: em28xx: fix assignment of the eeprom data Greg Kroah-Hartman
2013-08-13  6:33 ` [ 18/60] i2c: i2c-mxs: Use DMA mode even for small transfers Greg Kroah-Hartman
2013-08-13  6:33 ` [ 19/60] cpufreq: loongson2: fix regression related to clock management Greg Kroah-Hartman
2013-08-13  6:33 ` [ 20/60] cpufreq: rename ignore_nice as ignore_nice_load Greg Kroah-Hartman
2013-08-13  6:33 ` [ 21/60] ACPI / PM: Walk physical_node_list under physical_node_lock Greg Kroah-Hartman
2013-08-13  6:33 ` [ 22/60] tracing: Fix fields of struct trace_iterator that are zeroed by mistake Greg Kroah-Hartman
2013-08-13  6:33 ` [ 23/60] tracing: Make TRACE_ITER_STOP_ON_FREE stop the correct buffer Greg Kroah-Hartman
2013-08-13  6:33 ` [ 24/60] tracing: Use flag buffer_disabled for irqsoff tracer Greg Kroah-Hartman
2013-08-13  6:33 ` [ 25/60] tracing: Fix reset of time stamps during trace_clock changes Greg Kroah-Hartman
2013-08-13  6:33 ` [ 26/60] ALSA: usb-audio: do not trust too-big wMaxPacketSize values Greg Kroah-Hartman
2013-08-13  6:33 ` [ 27/60] ALSA: 6fire: fix DMA issues with URB transfer_buffer usage Greg Kroah-Hartman
2013-08-13  6:33 ` [ 28/60] Btrfs: release both paths before logging dir/changed extents Greg Kroah-Hartman
2013-08-13  6:33 ` [ 29/60] LOCKD: Dont call utsname()->nodename from nlmclnt_setlockargs Greg Kroah-Hartman
2013-08-13  6:33 ` [ 30/60] SUNRPC: Dont auto-disconnect from the local rpcbind socket Greg Kroah-Hartman
2013-08-13  6:33 ` [ 31/60] SUNRPC: If the rpcbind channel is disconnected, fail the call to unregister Greg Kroah-Hartman
2013-08-13  6:33 ` [ 32/60] virtio/console: Quit from splice_write if pipe->nrbufs is 0 Greg Kroah-Hartman
2013-08-13  6:34 ` Greg Kroah-Hartman [this message]
2013-08-13  6:34 ` [ 34/60] virtio: console: fix race with port unplug and open/close Greg Kroah-Hartman
2013-08-13  6:34 ` [ 35/60] virtio: console: fix race in port_fops_open() and port unplug Greg Kroah-Hartman
2013-08-13  6:34 ` [ 36/60] virtio: console: clean up port data immediately at time of unplug Greg Kroah-Hartman
2013-08-13  6:34 ` [ 37/60] virtio: console: fix raising SIGIO after port unplug Greg Kroah-Hartman
2013-08-13  6:34 ` [ 38/60] virtio: console: return -ENODEV on all read operations after unplug Greg Kroah-Hartman
2013-08-13  6:34 ` [ 39/60] drm/cirrus: Invalidate page tables when pinning a BO Greg Kroah-Hartman
2013-08-13  6:34 ` [ 40/60] drm/mgag200: " Greg Kroah-Hartman
2013-08-13  6:34 ` [ 41/60] drm/ast: invalidate " Greg Kroah-Hartman
2013-08-13  6:34 ` [ 42/60] drm: Dont pass negative delta to ktime_sub_ns() Greg Kroah-Hartman
2013-08-13  6:34 ` [ 43/60] drm/radeon: select audio dto based on encoder id for DCE3 Greg Kroah-Hartman
2013-08-13  6:34 ` [ 44/60] drm/radeon: stop sending invalid UVD destroy msg Greg Kroah-Hartman
2013-08-13  6:34 ` [ 45/60] ext4: allow the mount options nodelalloc and data=journal Greg Kroah-Hartman
2013-08-13  6:34 ` [ 46/60] ext4: fix mount/remount error messages for incompatible mount options Greg Kroah-Hartman
2013-08-13  6:34 ` [ 47/60] ext4: flush the extent status cache during EXT4_IOC_SWAP_BOOT Greg Kroah-Hartman
2013-08-13  6:34 ` [ 48/60] cifs: extend the buffer length enought for sprintf() using Greg Kroah-Hartman
2013-08-13  6:34 ` [ 49/60] cifs: dont instantiate new dentries in readdir for inodes that need to be revalidated immediately Greg Kroah-Hartman
2013-08-13  6:34 ` [ 50/60] zram: allow request end to coincide with disksize Greg Kroah-Hartman
2013-08-13  6:34 ` [ 51/60] usb: core: dont try to reset_device() a port that got just disconnected Greg Kroah-Hartman
2013-08-13  6:34 ` [ 52/60] debugfs: debugfs_remove_recursive() must not rely on list_empty(d_subdirs) Greg Kroah-Hartman
2013-08-13  6:34 ` [ 53/60] reiserfs: fix deadlock in umount Greg Kroah-Hartman
2013-08-13  6:34 ` [ 54/60] drm/i915: initialize gt_lock early with other spin locks Greg Kroah-Hartman
2013-08-13  6:34 ` [ 55/60] drm/radeon: fix halting UVD Greg Kroah-Hartman
2013-08-13  6:34 ` [ 56/60] drm/radeon: only save UVD bo when we have open handles Greg Kroah-Hartman
2013-08-13  6:34 ` [ 57/60] drm/radeon: always program the MC on startup Greg Kroah-Hartman
2013-08-13  6:34 ` [ 58/60] SCSI: nsp32: use mdelay instead of large udelay constants Greg Kroah-Hartman
2013-08-13  6:34 ` [ 59/60] mtd: omap2: allow bulding as a module Greg Kroah-Hartman
2013-08-13  6:34 ` [ 60/60] MIPS: Expose missing pci_io{map,unmap} declarations Greg Kroah-Hartman
2013-08-13 11:57 ` [ 00/60] 3.10.7-stable review Guenter Roeck
2013-08-13 17:57   ` Greg Kroah-Hartman
2013-08-13 17:18 ` Shuah Khan
2013-08-13 17:57   ` Greg Kroah-Hartman

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=20130813063336.785700982@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=amit.shah@redhat.com \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=rusty@rustcorp.com.au \
    --cc=stable@vger.kernel.org \
    --cc=yoshihiro.yunomae.ez@hitachi.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox