31 Oct, 2016
1 commit
-
Commit c6017e793b93 ("virtio: console: add locks around buffer removal
in port unplug path") added locking around the freeing of buffers in the
vq. However, when free_buf() is called with can_sleep = true and rproc
is enabled, it calls dma_free_coherent() directly, requiring interrupts
to be enabled. Currently a WARNING is triggered due to the spin locking
around free_buf, with a call stack like this:WARNING: CPU: 3 PID: 121 at ./include/linux/dma-mapping.h:433
free_buf+0x1a8/0x288
Call Trace:
[] show_stack+0x74/0xc0
[] dump_stack+0xd0/0x110
[] __warn+0xfc/0x130
[] warn_slowpath_null+0x2c/0x3c
[] free_buf+0x1a8/0x288
[] remove_port_data+0x50/0xac
[] unplug_port+0xb4/0x1bc
[] virtcons_remove+0xb0/0xfc
[] virtio_dev_remove+0x58/0xc0
[] __device_release_driver+0xac/0x134
[] device_release_driver+0x38/0x50
[] bus_remove_device+0xfc/0x130
[] device_del+0x17c/0x21c
[] device_unregister+0x24/0x38
[] unregister_virtio_device+0x28/0x44Fix this by restructuring the loops to allow the locks to only be taken
where it is necessary to protect the vqs, and release it while the
buffer is being freed.Fixes: c6017e793b93 ("virtio: console: add locks around buffer removal in port unplug path")
Cc: stable@vger.kernel.org
Signed-off-by: Matt Redfearn
Signed-off-by: Michael S. Tsirkin
12 Oct, 2016
1 commit
-
Kernel source files need not include explicitly
because the top Makefile forces to include it with:-include $(srctree)/include/linux/kconfig.h
This commit removes explicit includes except the following:
* arch/s390/include/asm/facilities_src.h
* tools/testing/radix-tree/linux/kernel.hThese two are used for host programs.
Link: http://lkml.kernel.org/r/1473656164-11929-1-git-send-email-yamada.masahiro@socionext.com
Signed-off-by: Masahiro Yamada
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
06 Oct, 2016
1 commit
-
Signed-off-by: Miklos Szeredi
Signed-off-by: Al Viro
10 Sep, 2016
1 commit
-
virtio_console uses a small DMA buffer for control requests. Move
that buffer into heap memory.Doing virtio DMA on the stack is normally okay on non-DMA-API virtio
systems (which is currently most of them), but it breaks completely
if the stack is virtually mapped.Tested by typing both directions using picocom aimed at /dev/hvc0.
Signed-off-by: Andy Lutomirski
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Amit Shah
25 May, 2015
1 commit
-
My static checker complains that this sprintf() can overflow but really
it can't. Just silence the warning by using snprintf().Signed-off-by: Dan Carpenter
Reviewed-by: Amit Shah
Signed-off-by: Greg Kroah-Hartman
03 Apr, 2015
1 commit
-
Use the normal return values for bool functions
Signed-off-by: Joe Perches
Reviewed-by: Amit Shah
Signed-off-by: Greg Kroah-Hartman
05 Mar, 2015
2 commits
-
when multiport is off, virtio console invokes config access from irq
context, config access is blocking on s390.
Fix this up by scheduling work from config irq - similar to what we do
for multiport configs.Signed-off-by: Michael S. Tsirkin
Reviewed-by: Amit Shah
Signed-off-by: Rusty Russell
Cc: stable@kernel.org -
when multiport is off, we don't initialize config work,
but we then cancel uninitialized control_work on freeze.Signed-off-by: Michael S. Tsirkin
Reviewed-by: Amit Shah
Signed-off-by: Rusty Russell
Cc: stable@kernel.org
11 Feb, 2015
1 commit
-
Strictly, it's only needed when we have features (size or multiport).
Signed-off-by: Rusty Russell
21 Jan, 2015
1 commit
-
Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/console needs config space access so make it
fail gracefully if not there.Signed-off-by: Michael S. Tsirkin
Signed-off-by: Rusty Russell
09 Dec, 2014
4 commits
-
CHECK drivers/char/virtio_console.c
drivers/char/virtio_console.c:687:36: warning: incorrect type in
argument 1 (different address spaces)
drivers/char/virtio_console.c:687:36: expected void [noderef]
*to
drivers/char/virtio_console.c:687:36: got char *out_buf
drivers/char/virtio_console.c:790:35: warning: incorrect type in
argument 2 (different address spaces)
drivers/char/virtio_console.c:790:35: expected char *out_buf
drivers/char/virtio_console.c:790:35: got char [noderef]
*ubuffill_readbuf is reused with both kernel and userspace pointers,
depending on value of to_user flag.Tag address parameter as __user, and cast to/from regular pointer type
when we know it's safe.Signed-off-by: Michael S. Tsirkin
-
Core activates this bit automatically now,
drop it from drivers that set it explicitly.Signed-off-by: Michael S. Tsirkin
-
Pretty straight-forward, just use accessors for all fields.
Signed-off-by: Michael S. Tsirkin
-
It seemed like a good idea to use bitmap for features
in struct virtio_device, but it's actually a pain,
and seems to become even more painful when we get more
than 32 feature bits. Just change it to a u32 for now.Based on patch by Rusty.
Suggested-by: David Hildenbrand
Signed-off-by: Rusty Russell
Signed-off-by: Cornelia Huck
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Cornelia Huck
13 Nov, 2014
1 commit
-
Commit f5866db6 (virtio_console: enable VQs early) tried to make
sure that DRIVER_OK was set when virtio_console started using its
virtqueues. Doing this in add_port(), however, means that we try
to set DRIVER_OK again when when a port is dynamically added after
the probe function is done.Let's move virtio_device_ready() to the probe function just before
trying to use the virtqueues instead. This is fine as nothing can
fail inbetween.Reported-by: Thomas Graf
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Cornelia HuckSigned-off-by: Michael S. Tsirkin
15 Oct, 2014
2 commits
-
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after resume returns, virtio console violated this
rule by adding inbufs, which causes the VQ to be used directly within
restore.To fix, call virtio_device_ready before using VQs.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Rusty Russell -
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, virtio console violated this
rule by adding inbufs, which causes the VQ to be used directly within
probe.To fix, call virtio_device_ready before using VQs.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Rusty Russell
27 Jul, 2014
1 commit
-
Fix checkpatch warning:
WARNING: debugfs_remove_recursive(NULL) is safe this check is probably not requiredCc: Arnd Bergmann
Cc: virtualization@lists.linux-foundation.org
Reviewed-by: Amit Shah
Signed-off-by: Fabian Frederick
Signed-off-by: Rusty Russell
02 Apr, 2014
1 commit
-
all pipe_buffer_operations have the same instances of those...
Signed-off-by: Al Viro
10 Feb, 2014
1 commit
-
While we are at it, don't do kmap() under kmap_atomic(), *especially*
for a page we'd allocated with GFP_KERNEL. It's spelled "page_address",
and had that been more than that, we'd have a real trouble - kmap_high()
can block, and doing that while holding kmap_atomic() is a Bad Idea(tm).Signed-off-by: Al Viro
29 Oct, 2013
1 commit
-
If virtqueue_get_buf() returns with a NULL pointer it should be verified
if the virtqueue is broken, in order to avoid loop calling cpu_relax().Signed-off-by: Heinz Graalfs
Signed-off-by: Rusty Russell
17 Oct, 2013
1 commit
-
This lets the transport do endian conversion if necessary, and insulates
the drivers from the difference.Most drivers can use the simple helpers virtio_cread() and virtio_cwrite().
Signed-off-by: Rusty Russell
23 Sep, 2013
1 commit
-
The freeze and restore functions defined in virtio drivers are used
for suspend and hibernate, so CONFIG_PM_SLEEP is more appropriate than
CONFIG_PM. This patch replace all CONFIG_PM with CONFIG_PM_SLEEP for
virtio drivers that implement freeze and restore callbacks.Signed-off-by: Aaron Lu
Reviewed-by: Amit Shah
Signed-off-by: Rusty Russell
09 Aug, 2013
2 commits
-
Remove the debugfs path before freeing port->name, to prevent a possible
use-after-free.Reported-by: Jason Wang
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
The next commit gets conflicts because it relies on patches which were
cc:stable and thus had to be merged into Linus' tree before the coming
merge window. So pull in master now.Signed-off-by: Rusty Russell
30 Jul, 2013
1 commit
-
The PTR_ERR(NULL) here is not useful.
Signed-off-by: Dan Carpenter
Reviewed-by: Amit Shah
Signed-off-by: Rusty Russell
29 Jul, 2013
8 commits
-
send_sigio_to_port() checks the value of guest_connected, which we
always modify under the inbuf_lock; make sure invocations of
send_sigio_to_port() have take the inbuf_lock around the call.Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
Port unplug can race with close() in port_fops_release().
port_fops_release() already takes the necessary locks, ensure
unplug_port() does that too.Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
The removal functions act on the vqs, and the vq operations need to be
locked.Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
If a port gets unplugged while a user is blocked on read(), -ENODEV is
returned. However, subsequent read()s returned 0, indicating there's no
host-side connection (but not indicating the device went away).This also happened when a port was unplugged and the user didn't have
any blocking operation pending. If the user didn't monitor the SIGIO
signal, they won't have a chance to find out if the port went away.Fix by returning -ENODEV on all read()s after the port gets unplugged.
write() already behaves this way.CC:
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
SIGIO should be sent when a port gets unplugged. It should only be sent
to prcesses that have the port opened, and have asked for SIGIO to be
delivered. We were clearing out guest_connected before calling
send_sigio_to_port(), resulting in a sigio not getting sent to
processes.Fix by setting guest_connected to false after invoking the sigio
function.CC:
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
We used to keep the port's char device structs and the /sys entries
around till the last reference to the port was dropped. This is
actually unnecessary, and resulted in buggy behaviour:1. Open port in guest
2. Hot-unplug port
3. Hot-plug a port with the same 'name' property as the unplugged oneThis resulted in hot-plug being unsuccessful, as a port with the same
name already exists (even though it was unplugged).This behaviour resulted in a warning message like this one:
-------------------8] ? warn_slowpath_common+0x87/0xc0
[] ? warn_slowpath_fmt+0x46/0x50
[] ? sysfs_add_one+0xc9/0x130
[] ? create_dir+0x68/0xb0
[] ? sysfs_create_dir+0x39/0x50
[] ? kobject_add_internal+0xb9/0x260
[] ? kobject_add_varg+0x38/0x60
[] ? kobject_add+0x44/0x70
[] ? get_device_parent+0xf4/0x1d0
[] ? device_add+0xc9/0x650-------------------8
Reported-by: chayang
Reported-by: YOGANANTH SUBRAMANIAN
Reported-by: FuXiangChun
Reported-by: Qunfang Zhang
Reported-by: Sibiao Luo
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
Between open() being called and processed, the port can be unplugged.
Check if this happened, and bail out.A simple test script to reproduce this is:
while true; do for i in $(seq 1 100); do echo $i > /dev/vport0p3; done; done;
This opens and closes the port a lot of times; unplugging the port while
this is happening triggers the bug.CC:
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell -
There's a window between find_port_by_devt() returning a port and us
taking a kref on the port, where the port could get unplugged. Fix it
by taking the reference in find_port_by_devt() itself.Problem reported and analyzed by Mateusz Guzik.
CC:
Reported-by: Mateusz Guzik
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell
23 Jul, 2013
2 commits
-
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.
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.BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
IP: [] 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:[] [] 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:
[] ? alloc_buf.isra.13+0x39/0xb0
[] ? virtcons_restore+0x100/0x100
[] __splice_from_pipe+0x7e/0x90
[] ? alloc_buf.isra.13+0x39/0xb0
[] port_fops_splice_write+0xe9/0x140
[] ? selinux_file_permission+0xc4/0x120
[] ? wait_port_writable+0x1b0/0x1b0
[] do_splice_from+0xa0/0x110
[] SyS_splice+0x5ff/0x6b0
[] 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 50 18 85 c0 0f 85 aa 00 00 00 48 89 da 4c 89 e6 4c 89 ff 41
RIP [] splice_from_pipe_feed+0x6f/0x130
RSP
CR2: 0000000000000018
---[ end trace 24572beb7764de59 ]---V2: Fix a locking problem for error
V3: Add Reviewed-by lines and stable@ line in sign-off areaSigned-off-by: Yoshihiro YUNOMAE
Reviewed-by: Amit Shah
Reviewed-by: Masami Hiramatsu
Cc: Amit Shah
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: stable@vger.kernel.org
Signed-off-by: Rusty Russell -
Quit from splice_write if pipe->nrbufs is 0 for avoiding oops in virtio-serial.
When an application was doing splice from a kernel buffer to virtio-serial on
a guest, the application received signal(SIGINT). This situation will normally
happen, but the kernel executed a kernel panic by oops as follows:BUG: unable to handle kernel paging request at ffff882071c8ef28
IP: [] sg_init_table+0x2f/0x50
PGD 1fac067 PUD 0
Oops: 0000 [#1] SMP
Modules linked in: lockd sunrpc bnep bluetooth rfkill 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 microcode virtio_balloon virtio_net pcspkr soundcore i2c_piix4 i2c_core uinput floppy
CPU: 1 PID: 908 Comm: trace-cmd Not tainted 3.10.0+ #49
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
task: ffff880071c64650 ti: ffff88007bf24000 task.ti: ffff88007bf24000
RIP: 0010:[] [] sg_init_table+0x2f/0x50
RSP: 0018:ffff88007bf25dd8 EFLAGS: 00010286
RAX: 0000001fffffffe0 RBX: ffff882071c8ef28 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880071c8ef48
RBP: ffff88007bf25de8 R08: ffff88007fd15d40 R09: ffff880071c8ef48
R10: ffffea0001c71040 R11: ffffffff8139c555 R12: 0000000000000000
R13: ffff88007506a3c0 R14: ffff88007c862500 R15: ffff880071c8ef00
FS: 00007f0a3646c740(0000) GS:ffff88007fd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff882071c8ef28 CR3: 000000007acbb000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Stack:
ffff880071c8ef48 ffff88007bf25e20 ffff88007bf25e88 ffffffff8139d6fa
ffff88007bf25e28 ffffffff8127a3f4 0000000000000000 0000000000000000
ffff880071c8ef48 0000100000000000 0000000000000003 ffff88007bf25e08
Call Trace:
[] port_fops_splice_write+0xaa/0x130
[] ? selinux_file_permission+0xc4/0x120
[] ? wait_port_writable+0x1b0/0x1b0
[] do_splice_from+0xa0/0x110
[] SyS_splice+0x5ff/0x6b0
[] system_call_fastpath+0x16/0x1b
Code: c1 e2 05 48 89 e5 48 83 ec 10 4c 89 65 f8 41 89 f4 31 f6 48 89 5d f0 48 89 fb e8 8d ce ff ff 41 8d 44 24 ff 48 c1 e0 05 48 01 c3 8b 03 48 83 e0 fe 48 83 c8 02 48 89 03 48 8b 5d f0 4c 8b 65
RIP [] sg_init_table+0x2f/0x50
RSP
CR2: ffff882071c8ef28
---[ end trace 86323505eb42ea8f ]---It seems to induce pagefault in sg_init_tabel() when pipe->nrbufs is equal to
zero. This may happen in a following situation:(1) The application normally does splice(read) from a kernel buffer, then does
splice(write) to virtio-serial.
(2) The application receives SIGINT when is doing splice(read), so splice(read)
is failed by EINTR. However, the application does not finish the operation.
(3) The application tries to do splice(write) without pipe->nrbufs.
(4) The virtio-console driver tries to touch scatterlist structure sgl in
sg_init_table(), but the region is out of bound.To avoid the case, a kernel should check whether pipe->nrbufs is empty or not
when splice_write is executed in the virtio-console driver.V3: Add Reviewed-by lines and stable@ line in sign-off area.
Signed-off-by: Yoshihiro YUNOMAE
Reviewed-by: Amit Shah
Reviewed-by: Masami Hiramatsu
Cc: Amit Shah
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: stable@vger.kernel.org
Signed-off-by: Rusty Russell
03 May, 2013
1 commit
-
Pull virtio & lguest updates from Rusty Russell:
"Lots of virtio work which wasn't quite ready for last merge window.Plus I dived into lguest again, reworking the pagetable code so we can
move the switcher page: our fixmaps sometimes take more than 2MB now..."Ugh. Annoying conflicts with the tcm_vhost -> vhost_scsi rename.
Hopefully correctly resolved.* tag 'virtio-next-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux: (57 commits)
caif_virtio: Remove bouncing email addresses
lguest: improve code readability in lg_cpu_start.
virtio-net: fill only rx queues which are being used
lguest: map Switcher below fixmap.
lguest: cache last cpu we ran on.
lguest: map Switcher text whenever we allocate a new pagetable.
lguest: don't share Switcher PTE pages between guests.
lguest: expost switcher_pages array (as lg_switcher_pages).
lguest: extract shadow PTE walking / allocating.
lguest: make check_gpte et. al return bool.
lguest: assume Switcher text is a single page.
lguest: rename switcher_page to switcher_pages.
lguest: remove RESERVE_MEM constant.
lguest: check vaddr not pgd for Switcher protection.
lguest: prepare to make SWITCHER_ADDR a variable.
virtio: console: replace EMFILE with EBUSY for already-open port
virtio-scsi: reset virtqueue affinity when doing cpu hotplug
virtio-scsi: introduce multiqueue support
virtio-scsi: push vq lock/unlock into virtscsi_vq_done
virtio-scsi: pass struct virtio_scsi to virtqueue completion function
...
15 Apr, 2013
1 commit
-
Returning EMFILE (process has too many open files) is incorrect to
indicate a port is already open by another process. Use EBUSY for that.This does change what we report to userspace, but I believe userspace
can look at it this way: it gets EBUSY, a new error code, instead of
EMFILE. It's still an error, and that's not changing.Reported-by: Mateusz Guzik
Signed-off-by: Amit Shah
Signed-off-by: Rusty Russell
08 Apr, 2013
1 commit
-
Those symbols only used within this file, and should be static.
Signed-off-by: Wei Yongjun
Acked-by: Amit Shah
Signed-off-by: Rusty Russell
30 Mar, 2013
1 commit
-
When multiple ovq operations are being performed (lots of open/close
operations on virtio_console fds), the __send_control_msg() function can
get confused without locking.A simple recipe to cause badness is:
* create a QEMU VM with two virtio-serial ports
* in the guest, do
while true;do echo abc >/dev/vport0p1;done
while true;do echo edf >/dev/vport0p2;doneIn one run, this caused a panic in __send_control_msg(). In another, I
gotvirtio_console virtio0: control-o:id 0 is not a head!
This also results repeated messages similar to these on the host:
qemu-kvm: virtio-serial-bus: Unexpected port id 478762112 for device virtio-serial-bus.0
qemu-kvm: virtio-serial-bus: Unexpected port id 478762368 for device virtio-serial-bus.0Reported-by: FuXiangChun
Signed-off-by: Amit Shah
Reviewed-by: Wanlong Gao
Reviewed-by: Asias He
Signed-off-by: Rusty Russell
Cc: stable@kernel.org