From: Tyler Hicks <[email protected]>
[ Upstream commit 60d4885710836595192c42d3e04b27551d30ec91 ]
Restore the behavior of locking mmap_sem for reading in
binder_alloc_free_page(), as was first done in commit 3013bf62b67a
("binder: reduce mmap_sem write-side lock"). That change was
inadvertently reverted by commit 5cec2d2e5839 ("binder: fix race between
munmap() and direct reclaim").
In addition, change the name of the label for the error path to
accurately reflect that we're taking the lock for reading.
Backporting note: This fix is only needed when *both* of the commits
mentioned above are applied. That's an unlikely situation since they
both landed during the development of v5.1 but only one of them is
targeted for stable.
Fixes: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim")
Signed-off-by: Tyler Hicks <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/android/binder_alloc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index a654ccfd1a222..21dc20c52cd4d 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -962,8 +962,8 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
mm = alloc->vma_vm_mm;
if (!mmget_not_zero(mm))
goto err_mmget;
- if (!down_write_trylock(&mm->mmap_sem))
- goto err_down_write_mmap_sem_failed;
+ if (!down_read_trylock(&mm->mmap_sem))
+ goto err_down_read_mmap_sem_failed;
vma = binder_alloc_get_vma(alloc);
list_lru_isolate(lru, item);
@@ -978,7 +978,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
trace_binder_unmap_user_end(alloc, index);
}
- up_write(&mm->mmap_sem);
+ up_read(&mm->mmap_sem);
mmput(mm);
trace_binder_unmap_kernel_start(alloc, index);
@@ -993,7 +993,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
mutex_unlock(&alloc->mutex);
return LRU_REMOVED_RETRY;
-err_down_write_mmap_sem_failed:
+err_down_read_mmap_sem_failed:
mmput_async(mm);
err_mmget:
err_page_already_freed:
--
2.20.1
Hello, Sasha!
On 2019-08-29 06:49:51, Sasha Levin wrote:
> From: Tyler Hicks <[email protected]>
>
> [ Upstream commit 60d4885710836595192c42d3e04b27551d30ec91 ]
>
> Restore the behavior of locking mmap_sem for reading in
> binder_alloc_free_page(), as was first done in commit 3013bf62b67a
> ("binder: reduce mmap_sem write-side lock"). That change was
> inadvertently reverted by commit 5cec2d2e5839 ("binder: fix race between
> munmap() and direct reclaim").
>
> In addition, change the name of the label for the error path to
> accurately reflect that we're taking the lock for reading.
>
> Backporting note: This fix is only needed when *both* of the commits
> mentioned above are applied. That's an unlikely situation since they
> both landed during the development of v5.1 but only one of them is
> targeted for stable.
This patch isn't meant to be applied to 4.19 since commit 3013bf62b67a
("binder: reduce mmap_sem write-side lock") was never brought back to
4.19.
Tyler
>
> Fixes: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim")
> Signed-off-by: Tyler Hicks <[email protected]>
> Acked-by: Todd Kjos <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> drivers/android/binder_alloc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index a654ccfd1a222..21dc20c52cd4d 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -962,8 +962,8 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
> mm = alloc->vma_vm_mm;
> if (!mmget_not_zero(mm))
> goto err_mmget;
> - if (!down_write_trylock(&mm->mmap_sem))
> - goto err_down_write_mmap_sem_failed;
> + if (!down_read_trylock(&mm->mmap_sem))
> + goto err_down_read_mmap_sem_failed;
> vma = binder_alloc_get_vma(alloc);
>
> list_lru_isolate(lru, item);
> @@ -978,7 +978,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
>
> trace_binder_unmap_user_end(alloc, index);
> }
> - up_write(&mm->mmap_sem);
> + up_read(&mm->mmap_sem);
> mmput(mm);
>
> trace_binder_unmap_kernel_start(alloc, index);
> @@ -993,7 +993,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
> mutex_unlock(&alloc->mutex);
> return LRU_REMOVED_RETRY;
>
> -err_down_write_mmap_sem_failed:
> +err_down_read_mmap_sem_failed:
> mmput_async(mm);
> err_mmget:
> err_page_already_freed:
> --
> 2.20.1
>
On Thu, Aug 29, 2019 at 10:13:20AM -0500, Tyler Hicks wrote:
> Hello, Sasha!
>
> On 2019-08-29 06:49:51, Sasha Levin wrote:
> > From: Tyler Hicks <[email protected]>
> >
> > [ Upstream commit 60d4885710836595192c42d3e04b27551d30ec91 ]
> >
> > Restore the behavior of locking mmap_sem for reading in
> > binder_alloc_free_page(), as was first done in commit 3013bf62b67a
> > ("binder: reduce mmap_sem write-side lock"). That change was
> > inadvertently reverted by commit 5cec2d2e5839 ("binder: fix race between
> > munmap() and direct reclaim").
> >
> > In addition, change the name of the label for the error path to
> > accurately reflect that we're taking the lock for reading.
> >
> > Backporting note: This fix is only needed when *both* of the commits
> > mentioned above are applied. That's an unlikely situation since they
> > both landed during the development of v5.1 but only one of them is
> > targeted for stable.
>
> This patch isn't meant to be applied to 4.19 since commit 3013bf62b67a
> ("binder: reduce mmap_sem write-side lock") was never brought back to
> 4.19.
>
> Tyler
>
> >
> > Fixes: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim")
But this commit is in 4.19.49
thanks,
greg k-h
On 2019-08-30 08:29:10, Greg Kroah-Hartman wrote:
> On Thu, Aug 29, 2019 at 10:13:20AM -0500, Tyler Hicks wrote:
> > Hello, Sasha!
> >
> > On 2019-08-29 06:49:51, Sasha Levin wrote:
> > > From: Tyler Hicks <[email protected]>
> > >
> > > [ Upstream commit 60d4885710836595192c42d3e04b27551d30ec91 ]
> > >
> > > Restore the behavior of locking mmap_sem for reading in
> > > binder_alloc_free_page(), as was first done in commit 3013bf62b67a
> > > ("binder: reduce mmap_sem write-side lock"). That change was
> > > inadvertently reverted by commit 5cec2d2e5839 ("binder: fix race between
> > > munmap() and direct reclaim").
> > >
> > > In addition, change the name of the label for the error path to
> > > accurately reflect that we're taking the lock for reading.
> > >
> > > Backporting note: This fix is only needed when *both* of the commits
> > > mentioned above are applied. That's an unlikely situation since they
> > > both landed during the development of v5.1 but only one of them is
> > > targeted for stable.
> >
> > This patch isn't meant to be applied to 4.19 since commit 3013bf62b67a
> > ("binder: reduce mmap_sem write-side lock") was never brought back to
> > 4.19.
> >
> > Tyler
> >
> > >
> > > Fixes: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim")
>
> But this commit is in 4.19.49
That's correct but commit 3013bf62b67a isn't present in 4.19.y. See the
"Backporting note" above.
Tyler
>
> thanks,
>
> greg k-h
On Fri, Aug 30, 2019 at 02:30:19AM -0500, Tyler Hicks wrote:
> On 2019-08-30 08:29:10, Greg Kroah-Hartman wrote:
> > On Thu, Aug 29, 2019 at 10:13:20AM -0500, Tyler Hicks wrote:
> > > Hello, Sasha!
> > >
> > > On 2019-08-29 06:49:51, Sasha Levin wrote:
> > > > From: Tyler Hicks <[email protected]>
> > > >
> > > > [ Upstream commit 60d4885710836595192c42d3e04b27551d30ec91 ]
> > > >
> > > > Restore the behavior of locking mmap_sem for reading in
> > > > binder_alloc_free_page(), as was first done in commit 3013bf62b67a
> > > > ("binder: reduce mmap_sem write-side lock"). That change was
> > > > inadvertently reverted by commit 5cec2d2e5839 ("binder: fix race between
> > > > munmap() and direct reclaim").
> > > >
> > > > In addition, change the name of the label for the error path to
> > > > accurately reflect that we're taking the lock for reading.
> > > >
> > > > Backporting note: This fix is only needed when *both* of the commits
> > > > mentioned above are applied. That's an unlikely situation since they
> > > > both landed during the development of v5.1 but only one of them is
> > > > targeted for stable.
> > >
> > > This patch isn't meant to be applied to 4.19 since commit 3013bf62b67a
> > > ("binder: reduce mmap_sem write-side lock") was never brought back to
> > > 4.19.
> > >
> > > Tyler
> > >
> > > >
> > > > Fixes: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim")
> >
> > But this commit is in 4.19.49
>
> That's correct but commit 3013bf62b67a isn't present in 4.19.y. See the
> "Backporting note" above.
Heh, I never read that part of the text, nevermind :)
greg k-h