From: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
To: Amit Shah <amit.shah@redhat.com>, linux-kernel@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
virtualization@lists.linux-foundation.org,
Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>,
yrl.pp-manager.tt@hitachi.com,
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Subject: [PATCH V2 2/2] [BUGFIX] virtio/console: Add pipe_lock/unlock for splice_write
Date: Fri, 19 Jul 2013 18:19:56 +0900 [thread overview]
Message-ID: <20130719091956.9698.30207.stgit@yunodevel> (raw)
In-Reply-To: <20130719091951.9698.95194.stgit@yunodevel>
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
Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
Cc: Amit Shah <amit.shah@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/char/virtio_console.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8722656..8a15af3 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -936,16 +936,21 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
* 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(struct pipe_inode_info *pipe,
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)
next prev parent reply other threads:[~2013-07-19 9:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-19 9:19 [PATCH V2 0/2] [BUGFIX] virtio/console: Fix two bugs of splice_write Yoshihiro YUNOMAE
2013-07-19 9:19 ` Yoshihiro YUNOMAE
2013-07-19 9:19 ` [PATCH V2 1/2] [BUGFIX] virtio/console: Quit from splice_write if pipe->nrbufs is 0 Yoshihiro YUNOMAE
2013-07-19 9:19 ` Yoshihiro YUNOMAE
2013-07-19 12:25 ` Masami Hiramatsu
2013-07-19 12:25 ` Masami Hiramatsu
2013-07-19 9:19 ` Yoshihiro YUNOMAE [this message]
2013-07-19 12:26 ` [PATCH V2 2/2] [BUGFIX] virtio/console: Add pipe_lock/unlock for splice_write Masami Hiramatsu
2013-07-19 12:26 ` Masami Hiramatsu
2013-07-19 9:19 ` Yoshihiro YUNOMAE
2013-07-19 10:05 ` [PATCH V2 0/2] [BUGFIX] virtio/console: Fix two bugs of splice_write Amit Shah
2013-07-19 10:05 ` Amit Shah
2013-07-22 1:05 ` Yoshihiro YUNOMAE
2013-07-22 1:05 ` Yoshihiro YUNOMAE
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=20130719091956.9698.30207.stgit@yunodevel \
--to=yoshihiro.yunomae.ez@hitachi.com \
--cc=amit.shah@redhat.com \
--cc=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=hidehiro.kawai.ez@hitachi.com \
--cc=linux-kernel@vger.kernel.org \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=stable@vger.kernel.org \
--cc=virtualization@lists.linux-foundation.org \
--cc=yrl.pp-manager.tt@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 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.