Re: possible deadlock in shmem_file_llseek

From: Joel Fernandes
Date: Mon Jan 29 2018 - 22:05:11 EST


On Wed, Jan 24, 2018 at 10:40 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> On Wed, Jan 24, 2018 at 6:47 PM, Joel Fernandes <joelaf@xxxxxxxxxx> wrote:
>>
>> #syz test: https://github.com/joelagnel/linux.git test-ashmem

Here's an updated patch. Just wanted to test it once more.

thanks,

- Joel
From e7d73362ed378499795f27e2da6184d0b242b89c Mon Sep 17 00:00:00 2001
From: Joel Fernandes <joelaf@xxxxxxxxxx>
Date: Wed, 24 Jan 2018 09:47:23 -0800
Subject: [PATCH] ashmem: Fix lockdep issue during llseek

ashmem_mutex create a chain of dependencies like so:

(1)
mmap syscall ->
mmap_sem -> (acquired)
ashmem_mmap
ashmem_mutex (try to acquire)
(block)

(2)
llseek syscall ->
ashmem_llseek ->
ashmem_mutex -> (acquired)
inode_lock ->
inode->i_rwsem (try to acquire)
(block)

(3)
getdents ->
iterate_dir ->
inode_lock ->
inode->i_rwsem (acquired)
copy_to_user ->
mmap_sem (try to acquire)

There is a lock ordering created between mmap_sem and inode->i_rwsem causing a
lockdep splat [2] during a syzcaller test, this patch fixes the issue by
removing the use of the mutex.

The mutex isn't needed as llseeks are synchronized with other llseeks
and reads due to fdget_pos as Al mentioned [1]. Further asma->file
and asma->size don't change once they're setup, and there's a unique
asma->file per file.

[1] https://patchwork.kernel.org/patch/10185031/
[2] https://lkml.org/lkml/2018/1/10/48

Cc: Todd Kjos <tkjos@xxxxxxxxxx>
Cc: Arve Hjonnevag <arve@xxxxxxxxxxx>
Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Reported-by: syzbot+8ec30bb7bf1a981a2012@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Joel Fernandes <joelaf@xxxxxxxxxx>
---
drivers/staging/android/ashmem.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 0f695df14c9d..8c9c69a93fa4 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -331,8 +331,6 @@ static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin)
struct ashmem_area *asma = file->private_data;
int ret;

- mutex_lock(&ashmem_mutex);
-
if (asma->size == 0) {
ret = -EINVAL;
goto out;
@@ -351,7 +349,6 @@ static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin)
file->f_pos = asma->file->f_pos;

out:
- mutex_unlock(&ashmem_mutex);
return ret;
}

--
2.16.0.rc1.238.g530d649a79-goog