search for: port_fops_release

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 <amit.shah at redhat.com> --- drivers/char/virtio_console.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_consol...

...hah <amit.shah at redhat.com> >> >> How can userspace access the port now? By the time we are cleaning up >> buffers, there should be no possibility of such accesses. > > close(), can happen when the port is being unplugged. We're just > making sure here that port_fops_release() and unplug_port() don't try > to free up the same data at the same time. Why doesn't reference counting help us here? Surely the last one should clean up? >> The number of bugfixes here is deeply disturbing. > > Yes, the first three fix a bug - close() after unplug. How...

...hah <amit.shah at redhat.com> >> >> How can userspace access the port now? By the time we are cleaning up >> buffers, there should be no possibility of such accesses. > > close(), can happen when the port is being unplugged. We're just > making sure here that port_fops_release() and unplug_port() don't try > to free up the same data at the same time. Why doesn't reference counting help us here? Surely the last one should clean up? >> The number of bugfixes here is deeply disturbing. > > Yes, the first three fix a bug - close() after unplug. How...

Amit Shah <amit.shah at redhat.com> writes: > The removal functions act on the vqs, and the vq operations need to be > locked. > > Signed-off-by: Amit Shah <amit.shah at redhat.com> How can userspace access the port now? By the time we are cleaning up buffers, there should be no possibility of such accesses. The number of bugfixes here is deeply disturbing. I wonder if

Amit Shah <amit.shah at redhat.com> writes: > The removal functions act on the vqs, and the vq operations need to be > locked. > > Signed-off-by: Amit Shah <amit.shah at redhat.com> How can userspace access the port now? By the time we are cleaning up buffers, there should be no possibility of such accesses. The number of bugfixes here is deeply disturbing. I wonder if

...w code is sufficiently different, so I'd rather it bakes for a while, and I ensure there are no regressions wrt the tests I have so far for a while as well. Hopefully this will be in time for the next merge window. There's one use-after-free I spotted after sending the first two versions: port_fops_release() calls send_control_msg(), which spins till the host acknowledges receipt of the buffer. While it's spinning, if the device gets unplugged, the vqs go away, and the spinning function never progresses, causing a softlockup. This is difficult to reproduce -- the host usually acknowledges the b...

...w code is sufficiently different, so I'd rather it bakes for a while, and I ensure there are no regressions wrt the tests I have so far for a while as well. Hopefully this will be in time for the next merge window. There's one use-after-free I spotted after sending the first two versions: port_fops_release() calls send_control_msg(), which spins till the host acknowledges receipt of the buffer. While it's spinning, if the device gets unplugged, the vqs go away, and the spinning function never progresses, causing a softlockup. This is difficult to reproduce -- the host usually acknowledges the b...

...> Signed-off-by: Amit Shah <amit.shah at redhat.com> > > How can userspace access the port now? By the time we are cleaning up > buffers, there should be no possibility of such accesses. close(), can happen when the port is being unplugged. We're just making sure here that port_fops_release() and unplug_port() don't try to free up the same data at the same time. > The number of bugfixes here is deeply disturbing. Yes, the first three fix a bug - close() after unplug. However, the others are inadequate locking fixes which I noticed while fixing that bug. Port unplug isn'...

...oll_table *wait) { @@ -1109,7 +1111,7 @@ static const struct file_operations port_fops = { .open = port_fops_open, .read = port_fops_read, .write = port_fops_write, - .splice_write = port_fops_splice_write, + SPLICE_WRITE_INIT(port_fops_splice_write) .poll = port_fops_poll, .release = port_fops_release, .fasync = port_fops_fasync, -- 1.9.1

...oll_table *wait) { @@ -1109,7 +1111,7 @@ static const struct file_operations port_fops = { .open = port_fops_open, .read = port_fops_read, .write = port_fops_write, - .splice_write = port_fops_splice_write, + SPLICE_WRITE_INIT(port_fops_splice_write) .poll = port_fops_poll, .release = port_fops_release, .fasync = port_fops_fasync, -- 1.9.1

From: Hans de Goede <hdegoede at redhat.com> I found this while working on a Linux agent for spice, the symptom I was seeing was select blocking on the spice vdagent virtio serial port even though there were messages queued up there. virtio_console's port_fops_poll checks port->inbuf != NULL to determine if read won't block. However if an application reads enough bytes from

From: Hans de Goede <hdegoede at redhat.com> I found this while working on a Linux agent for spice, the symptom I was seeing was select blocking on the spice vdagent virtio serial port even though there were messages queued up there. virtio_console's port_fops_poll checks port->inbuf != NULL to determine if read won't block. However if an application reads enough bytes from

...); - buf = kmalloc(count, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -720,7 +779,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf, goto out; free_buf: - kfree(buf); + free_databuf(vdev, buf_size, buf); out: return ret; } @@ -767,6 +826,7 @@ static int port_fops_release(struct inode *inode, struct file *filp) spin_unlock_irq(&port->inbuf_lock); spin_lock_irq(&port->outvq_lock); + reclaim_consumed_buffers(port); spin_unlock_irq(&port->outvq_lock); @@ -918,7 +978,8 @@ static void resize_console(struct port *port) return; vdev...

...); - buf = kmalloc(count, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -720,7 +779,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf, goto out; free_buf: - kfree(buf); + free_databuf(vdev, buf_size, buf); out: return ret; } @@ -767,6 +826,7 @@ static int port_fops_release(struct inode *inode, struct file *filp) spin_unlock_irq(&port->inbuf_lock); spin_lock_irq(&port->outvq_lock); + reclaim_consumed_buffers(port); spin_unlock_irq(&port->outvq_lock); @@ -918,7 +978,8 @@ static void resize_console(struct port *port) return; vdev...

Hey Rusty, This patchset is on top of the previous one. It sends a SIGIO signal to apps that request signals for host activity. SIGIO is sent on host connect, disconnect as well as hot-unplug (which can be seen as a special case of host disconnect). Tested using several testcases in the test-virtserial repo: http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git Please apply.

Hey Rusty, This patchset is on top of the previous one. It sends a SIGIO signal to apps that request signals for host activity. SIGIO is sent on host connect, disconnect as well as hot-unplug (which can be seen as a special case of host disconnect). Tested using several testcases in the test-virtserial repo: http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git Please apply.

Hi Rusty, The main thing in these patches is the introduction of injecting SIGIO on host-side connect/disconnect events and when new data is available for ports. The first two patches fix bugs that I haven't seen, but look like the right thing to do. These have been tested extensively using the test-virtserial test suite. Please apply, Amit. Amit Shah (4): virtio: console: Un-block

Hi Rusty, The main thing in these patches is the introduction of injecting SIGIO on host-side connect/disconnect events and when new data is available for ports. The first two patches fix bugs that I haven't seen, but look like the right thing to do. These have been tested extensively using the test-virtserial test suite. Please apply, Amit. Amit Shah (4): virtio: console: Un-block

Hello, This series fixes a few bugs and races with port unplug and the various file operations: read(), write(), close() and poll(). There still might be more races lurking, but testing this series looks good to at least solve the easily-triggerable ones. I've run the virtio-serial testsuite and a few open/close/unplug tests, and haven't seen any badness. I've marked these patches

Hello, This series fixes a few bugs and races with port unplug and the various file operations: read(), write(), close() and poll(). There still might be more races lurking, but testing this series looks good to at least solve the easily-triggerable ones. I've run the virtio-serial testsuite and a few open/close/unplug tests, and haven't seen any badness. I've marked these patches