LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] android: binder: Fix a possible data race in binder_alloc_mmap_handler
@ 2018-05-08  9:06 Jia-Ju Bai
  2018-05-08 13:38 ` Martijn Coenen
  0 siblings, 1 reply; 2+ messages in thread
From: Jia-Ju Bai @ 2018-05-08  9:06 UTC (permalink / raw)
  To: gregkh, arve, tkjos, maco; +Cc: devel, linux-kernel, Jia-Ju Bai

The write operations to "alloc->buffer" are protected by
the lock on line 679 and 730, but the read operation to
this data on line 712 is not protected by the lock.
Thus, there may exist a data race for "alloc->buffer".

To fix this data race, the read operation to "alloc->buffer" 
should be also protected by the lock.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 drivers/android/binder_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 5a426c877dfb..596acc3a84e4 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -709,7 +709,9 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
 		goto err_alloc_buf_struct_failed;
 	}
 
+	mutex_lock(&binder_alloc_mmap_lock);
 	buffer->data = alloc->buffer;
+	mutex_unlock(&binder_alloc_mmap_lock);
 	list_add(&buffer->entry, &alloc->buffers);
 	buffer->free = 1;
 	binder_insert_free_buffer(alloc, buffer);
-- 
2.17.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] android: binder: Fix a possible data race in binder_alloc_mmap_handler
  2018-05-08  9:06 [PATCH] android: binder: Fix a possible data race in binder_alloc_mmap_handler Jia-Ju Bai
@ 2018-05-08 13:38 ` Martijn Coenen
  0 siblings, 0 replies; 2+ messages in thread
From: Martijn Coenen @ 2018-05-08 13:38 UTC (permalink / raw)
  To: Jia-Ju Bai
  Cc: open list:ANDROID DRIVERS, Greg KH, Arve Hjønnevåg,
	Todd Kjos, LKML

On Tue, May 8, 2018 at 2:06 AM, Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> The write operations to "alloc->buffer" are protected by
> the lock on line 679 and 730, but the read operation to
> this data on line 712 is not protected by the lock.
> Thus, there may exist a data race for "alloc->buffer".

It's read by the same thread that just wrote it, there is no data race.

The locks at line 679 and 730 protect against multiple threads calling
mmap() at the same time.

>
> To fix this data race, the read operation to "alloc->buffer"
> should be also protected by the lock.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
>  drivers/android/binder_alloc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 5a426c877dfb..596acc3a84e4 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -709,7 +709,9 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
>                 goto err_alloc_buf_struct_failed;
>         }
>
> +       mutex_lock(&binder_alloc_mmap_lock);
>         buffer->data = alloc->buffer;
> +       mutex_unlock(&binder_alloc_mmap_lock);
>         list_add(&buffer->entry, &alloc->buffers);
>         buffer->free = 1;
>         binder_insert_free_buffer(alloc, buffer);
> --
> 2.17.0
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-05-08 13:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08  9:06 [PATCH] android: binder: Fix a possible data race in binder_alloc_mmap_handler Jia-Ju Bai
2018-05-08 13:38 ` Martijn Coenen

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).