diff options
author | Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com> | 2013-07-22 22:00:49 -0400 |
---|---|---|
committer | Rusty Russell <rusty@rustcorp.com.au> | 2013-07-22 22:45:26 -0400 |
commit | 2b4fbf029dff5a28d9bf646346dea891ec43398a (patch) | |
tree | bec4be587fc4ffc282a8b11d9a4c4a817291d0ed /drivers/char/virtio_console.c | |
parent | 68c034fefe20eaf7d5569aae84584b07987ce50a (diff) |
virtio/console: Add pipe_lock/unlock for splice_write
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>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Diffstat (limited to 'drivers/char/virtio_console.c')
-rw-r--r-- | drivers/char/virtio_console.c | 20 |
1 files changed, 15 insertions, 5 deletions
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 8722656cdebf..8a15af3e1a9d 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, | |||
936 | * pipe->nrbufs == 0 means there are no data to transfer, | 936 | * pipe->nrbufs == 0 means there are no data to transfer, |
937 | * so this returns just 0 for no data. | 937 | * so this returns just 0 for no data. |
938 | */ | 938 | */ |
939 | if (!pipe->nrbufs) | 939 | pipe_lock(pipe); |
940 | return 0; | 940 | if (!pipe->nrbufs) { |
941 | ret = 0; | ||
942 | goto error_out; | ||
943 | } | ||
941 | 944 | ||
942 | ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK); | 945 | ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK); |
943 | if (ret < 0) | 946 | if (ret < 0) |
944 | return ret; | 947 | goto error_out; |
945 | 948 | ||
946 | buf = alloc_buf(port->out_vq, 0, pipe->nrbufs); | 949 | buf = alloc_buf(port->out_vq, 0, pipe->nrbufs); |
947 | if (!buf) | 950 | if (!buf) { |
948 | return -ENOMEM; | 951 | ret = -ENOMEM; |
952 | goto error_out; | ||
953 | } | ||
949 | 954 | ||
950 | sgl.n = 0; | 955 | sgl.n = 0; |
951 | sgl.len = 0; | 956 | sgl.len = 0; |
@@ -953,12 +958,17 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe, | |||
953 | sgl.sg = buf->sg; | 958 | sgl.sg = buf->sg; |
954 | sg_init_table(sgl.sg, sgl.size); | 959 | sg_init_table(sgl.sg, sgl.size); |
955 | ret = __splice_from_pipe(pipe, &sd, pipe_to_sg); | 960 | ret = __splice_from_pipe(pipe, &sd, pipe_to_sg); |
961 | pipe_unlock(pipe); | ||
956 | if (likely(ret > 0)) | 962 | if (likely(ret > 0)) |
957 | ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true); | 963 | ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true); |
958 | 964 | ||
959 | if (unlikely(ret <= 0)) | 965 | if (unlikely(ret <= 0)) |
960 | free_buf(buf, true); | 966 | free_buf(buf, true); |
961 | return ret; | 967 | return ret; |
968 | |||
969 | error_out: | ||
970 | pipe_unlock(pipe); | ||
971 | return ret; | ||
962 | } | 972 | } |
963 | 973 | ||
964 | static unsigned int port_fops_poll(struct file *filp, poll_table *wait) | 974 | static unsigned int port_fops_poll(struct file *filp, poll_table *wait) |