* [patch] goldfish: locking bugs in goldfish_pipe_read_write()
@ 2016-02-02 9:48 Dan Carpenter
2016-02-02 11:41 ` Christoffer Dall
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2016-02-02 9:48 UTC (permalink / raw)
To: Greg Kroah-Hartman, Christoffer Dall
Cc: Alan Cox, Jin Qian, Greg Hackmann, Yu Ning, Peter Senna Tschudin,
Alex Bennée, Jason Hu, linux-kernel, kernel-janitors
We recently messed up the error handling here. We can return with the
pipe->lock held or sometimes we unlock twice by mistake.
Fixes: 2f3be88237a3 ('goldfish_pipe: Pin pages to memory while copying and other cleanups')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index e3fab9a..839df4a 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -313,7 +313,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
!is_write, 0, &page, NULL);
up_read(¤t->mm->mmap_sem);
if (ret < 0)
- return ret;
+ break;
if (dev->version) {
/* Device version 1 or newer (qemu-android) expects the
@@ -400,22 +400,16 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
while (test_bit(wakeBit, &pipe->flags)) {
if (wait_event_interruptible(
pipe->wake_queue,
- !test_bit(wakeBit, &pipe->flags))) {
- ret = -ERESTARTSYS;
- break;
- }
-
- if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags)) {
- ret = -EIO;
- break;
- }
+ !test_bit(wakeBit, &pipe->flags)))
+ return -ERESTARTSYS;
+
+ if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags))
+ return -EIO;
}
/* Try to re-acquire the lock */
- if (mutex_lock_interruptible(&pipe->lock)) {
- ret = -ERESTARTSYS;
- break;
- }
+ if (mutex_lock_interruptible(&pipe->lock))
+ return -ERESTARTSYS;
}
mutex_unlock(&pipe->lock);
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [patch] goldfish: locking bugs in goldfish_pipe_read_write()
2016-02-02 9:48 [patch] goldfish: locking bugs in goldfish_pipe_read_write() Dan Carpenter
@ 2016-02-02 11:41 ` Christoffer Dall
0 siblings, 0 replies; 2+ messages in thread
From: Christoffer Dall @ 2016-02-02 11:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Alan Cox, Jin Qian, Greg Hackmann, Yu Ning,
Peter Senna Tschudin, Alex Bennée, Jason Hu, linux-kernel,
kernel-janitors
On Tue, Feb 02, 2016 at 12:48:09PM +0300, Dan Carpenter wrote:
> We recently messed up the error handling here. We can return with the
> pipe->lock held or sometimes we unlock twice by mistake.
>
> Fixes: 2f3be88237a3 ('goldfish_pipe: Pin pages to memory while copying and other cleanups')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
> index e3fab9a..839df4a 100644
> --- a/drivers/platform/goldfish/goldfish_pipe.c
> +++ b/drivers/platform/goldfish/goldfish_pipe.c
> @@ -313,7 +313,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
> !is_write, 0, &page, NULL);
> up_read(¤t->mm->mmap_sem);
> if (ret < 0)
> - return ret;
> + break;
>
> if (dev->version) {
> /* Device version 1 or newer (qemu-android) expects the
> @@ -400,22 +400,16 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
> while (test_bit(wakeBit, &pipe->flags)) {
> if (wait_event_interruptible(
> pipe->wake_queue,
> - !test_bit(wakeBit, &pipe->flags))) {
> - ret = -ERESTARTSYS;
> - break;
> - }
> -
> - if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags)) {
> - ret = -EIO;
> - break;
> - }
> + !test_bit(wakeBit, &pipe->flags)))
> + return -ERESTARTSYS;
> +
> + if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags))
> + return -EIO;
> }
>
> /* Try to re-acquire the lock */
> - if (mutex_lock_interruptible(&pipe->lock)) {
> - ret = -ERESTARTSYS;
> - break;
> - }
> + if (mutex_lock_interruptible(&pipe->lock))
> + return -ERESTARTSYS;
> }
> mutex_unlock(&pipe->lock);
>
yeah, that was pretty broken, thanks for fixing this.
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-02-02 11:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-02 9:48 [patch] goldfish: locking bugs in goldfish_pipe_read_write() Dan Carpenter
2016-02-02 11:41 ` Christoffer Dall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).