Skip to content

Commit 98c9710

Browse files
Yoshihiro YUNOMAEgregkh
authored andcommitted
virtio/console: Add pipe_lock/unlock for splice_write
commit 2b4fbf0 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 <[email protected]> Reviewed-by: Amit Shah <[email protected]> Reviewed-by: Masami Hiramatsu <[email protected]> Cc: Amit Shah <[email protected]> Cc: Arnd Bergmann <[email protected]> Signed-off-by: Rusty Russell <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 81b80fb commit 98c9710

File tree

1 file changed

+15
-5
lines changed

1 file changed

+15
-5
lines changed

drivers/char/virtio_console.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -936,29 +936,39 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
936936
* pipe->nrbufs == 0 means there are no data to transfer,
937937
* so this returns just 0 for no data.
938938
*/
939-
if (!pipe->nrbufs)
940-
return 0;
939+
pipe_lock(pipe);
940+
if (!pipe->nrbufs) {
941+
ret = 0;
942+
goto error_out;
943+
}
941944

942945
ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
943946
if (ret < 0)
944-
return ret;
947+
goto error_out;
945948

946949
buf = alloc_buf(port->out_vq, 0, pipe->nrbufs);
947-
if (!buf)
948-
return -ENOMEM;
950+
if (!buf) {
951+
ret = -ENOMEM;
952+
goto error_out;
953+
}
949954

950955
sgl.n = 0;
951956
sgl.len = 0;
952957
sgl.size = pipe->nrbufs;
953958
sgl.sg = buf->sg;
954959
sg_init_table(sgl.sg, sgl.size);
955960
ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
961+
pipe_unlock(pipe);
956962
if (likely(ret > 0))
957963
ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true);
958964

959965
if (unlikely(ret <= 0))
960966
free_buf(buf, true);
961967
return ret;
968+
969+
error_out:
970+
pipe_unlock(pipe);
971+
return ret;
962972
}
963973

964974
static unsigned int port_fops_poll(struct file *filp, poll_table *wait)

0 commit comments

Comments
 (0)