search for: port_fops_read

...++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 6bf0df3..a39702a 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -749,6 +749,10 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf, > > port = filp->private_data; > > + /* Port is hot-unplugged. */ > + if (!port->guest_connected) > + return -ENODEV; > + What if the port is hot-unplugged after this check? Should we serialize the hot plug with inbuf_lock her...

...++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 6bf0df3..a39702a 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -749,6 +749,10 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf, > > port = filp->private_data; > > + /* Port is hot-unplugged. */ > + if (!port->guest_connected) > + return -ENODEV; > + What if the port is hot-unplugged after this check? Should we serialize the hot plug with inbuf_lock her...

...> port = filp->private_data; > + if (!port->guest_connected) { > + /* Port was unplugged before we could proceed */ > + return POLLHUP; > + } > poll_wait(filp, &port->waitqueue, wait); > > if (!port->guest_connected) { Looks still racy here. Unlike port_fops_read() which check will_read_block(). If unplug happens after the check but before the poll_wait(), caller will be blocked forever.

...> port = filp->private_data; > + if (!port->guest_connected) { > + /* Port was unplugged before we could proceed */ > + return POLLHUP; > + } > poll_wait(filp, &port->waitqueue, wait); > > if (!port->guest_connected) { Looks still racy here. Unlike port_fops_read() which check will_read_block(). If unplug happens after the check but before the poll_wait(), caller will be blocked forever.

...gged before we could proceed */ >> >>> + return POLLHUP; >> >>> + } >> >>> poll_wait(filp, &port->waitqueue, wait); >> >>> >> >>> if (!port->guest_connected) { >> >> Looks still racy here. Unlike port_fops_read() which check >> >> will_read_block(). If unplug happens after the check but before the >> >> poll_wait(), caller will be blocked forever. >> > unplug_port() calls wake_up_interruptible on the waitqueue. >> >> I mean the following cases: > > (form...

...gged before we could proceed */ >> >>> + return POLLHUP; >> >>> + } >> >>> poll_wait(filp, &port->waitqueue, wait); >> >>> >> >>> if (!port->guest_connected) { >> >> Looks still racy here. Unlike port_fops_read() which check >> >> will_read_block(). If unplug happens after the check but before the >> >> poll_wait(), caller will be blocked forever. >> > unplug_port() calls wake_up_interruptible on the waitqueue. >> >> I mean the following cases: > > (form...

...t;guest_connected) { >>> + /* Port was unplugged before we could proceed */ >>> + return POLLHUP; >>> + } >>> poll_wait(filp, &port->waitqueue, wait); >>> >>> if (!port->guest_connected) { >> Looks still racy here. Unlike port_fops_read() which check >> will_read_block(). If unplug happens after the check but before the >> poll_wait(), caller will be blocked forever. > unplug_port() calls wake_up_interruptible on the waitqueue. I mean the following cases: CPU0: CPU1:...

...t;guest_connected) { >>> + /* Port was unplugged before we could proceed */ >>> + return POLLHUP; >>> + } >>> poll_wait(filp, &port->waitqueue, wait); >>> >>> if (!port->guest_connected) { >> Looks still racy here. Unlike port_fops_read() which check >> will_read_block(). If unplug happens after the check but before the >> poll_wait(), caller will be blocked forever. > unplug_port() calls wake_up_interruptible on the waitqueue. I mean the following cases: CPU0: CPU1:...

...vers/char/virtio_console.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 6bf0df3..a39702a 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -749,6 +749,10 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf, port = filp->private_data; + /* Port is hot-unplugged. */ + if (!port->guest_connected) + return -ENODEV; + if (!port_has_data(port)) { /* * If nothing's connected on the host just return 0 in @@ -765,7 +769,7 @@ static ssize_t port_fo...

...vers/char/virtio_console.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 3435348..2b68075 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -749,6 +749,10 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf, port = filp->private_data; + /* Port is hot-unplugged. */ + if (!port->guest_connected) + return -ENODEV; + if (!port_has_data(port)) { /* * If nothing's connected on the host just return 0 in @@ -765,7 +769,7 @@ static ssize_t port_fo...

...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 inbuf through port_fops_read, to empty the current port->inbuf, port->inbuf will be NULL even though there may be buffers left in the virtqueue. This causes poll() to block even though there is data to be read, this patch fixes this by using the alredy defined will_read_block utility function instead of the port->inb...

...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 inbuf through port_fops_read, to empty the current port->inbuf, port->inbuf will be NULL even though there may be buffers left in the virtqueue. This causes poll() to block even though there is data to be read, this patch fixes this by using the alredy defined will_read_block utility function instead of the port->inb...

...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 inbuf through port_fops_read, to empty the current port->inbuf, port->inbuf will be NULL even though there may be buffers left in the virtqueue. This causes poll() to block even though there is data to be read, this patch fixes this by using will_read_block(port) instead of the port->inbuf != NULL check. Signed-off-...

...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 inbuf through port_fops_read, to empty the current port->inbuf, port->inbuf will be NULL even though there may be buffers left in the virtqueue. This causes poll() to block even though there is data to be read, this patch fixes this by using will_read_block(port) instead of the port->inbuf != NULL check. Signed-off-...

...t;>>>> + return POLLHUP; >>>>>>> + } >>>>>>> poll_wait(filp, &port->waitqueue, wait); >>>>>>> >>>>>>> if (!port->guest_connected) { >>>>>> Looks still racy here. Unlike port_fops_read() which check >>>>>> will_read_block(). If unplug happens after the check but before the >>>>>> poll_wait(), caller will be blocked forever. >>>>> unplug_port() calls wake_up_interruptible on the waitqueue. >>>> I mean the following cas...

...t;>>>> + return POLLHUP; >>>>>>> + } >>>>>>> poll_wait(filp, &port->waitqueue, wait); >>>>>>> >>>>>>> if (!port->guest_connected) { >>>>>> Looks still racy here. Unlike port_fops_read() which check >>>>>> will_read_block(). If unplug happens after the check but before the >>>>>> poll_wait(), caller will be blocked forever. >>>>> unplug_port() calls wake_up_interruptible on the waitqueue. >>>> I mean the following cas...

Hello, This series fixes a few 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 for

Hello, This series fixes a few 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 for

...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 inbuf through port_fops_read, to empty the current port->inbuf, port->inbuf will be NULL even though there may be buffers left in the virtqueue. This causes poll() to block even though there is data ready to be read, this patch fixes this by using port_has_data(port) instead of the port->inbuf != NULL check. Signed-...

...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 inbuf through port_fops_read, to empty the current port->inbuf, port->inbuf will be NULL even though there may be buffers left in the virtqueue. This causes poll() to block even though there is data ready to be read, this patch fixes this by using port_has_data(port) instead of the port->inbuf != NULL check. Signed-...