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 unlocking the mutex earlier. Functionally that's Ok since
we don't need to protect vfs_llseek.
[1] https://patchwork.kernel.org/patch/10185031/
[2] https://lkml.org/lkml/2018/1/10/48
Cc: Todd Kjos <[email protected]>
Cc: Arve Hjonnevag <[email protected]>
Cc: Greg Hackmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Reported-by: [email protected]
Signed-off-by: Joel Fernandes <[email protected]>
---
drivers/staging/android/ashmem.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 372ce9913e6d..6921f86b4aa1 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -334,24 +334,23 @@ static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin)
mutex_lock(&ashmem_mutex);
if (asma->size == 0) {
- ret = -EINVAL;
- goto out;
+ mutex_unlock(&ashmem_mutex);
+ return -EINVAL;
}
if (!asma->file) {
- ret = -EBADF;
- goto out;
+ mutex_unlock(&ashmem_mutex);
+ return -EBADF;
}
+ mutex_unlock(&ashmem_mutex);
+
ret = vfs_llseek(asma->file, offset, origin);
if (ret < 0)
- goto out;
+ return ret;
/** Copy f_pos from backing file, since f_ops->llseek() sets it */
file->f_pos = asma->file->f_pos;
-
-out:
- mutex_unlock(&ashmem_mutex);
return ret;
}
--
2.16.0.rc1.238.g530d649a79-goog
On Mon, Feb 12, 2018 at 05:01:25PM -0800, Joel Fernandes wrote:
> 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 unlocking the mutex earlier. Functionally that's Ok since
> we don't need to protect vfs_llseek.
>
> [1] https://patchwork.kernel.org/patch/10185031/
> [2] https://lkml.org/lkml/2018/1/10/48
>
> Cc: Todd Kjos <[email protected]>
> Cc: Arve Hjonnevag <[email protected]>
> Cc: Greg Hackmann <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Reported-by: [email protected]
> Signed-off-by: Joel Fernandes <[email protected]>
> ---
> drivers/staging/android/ashmem.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
Please always properly version your patches, and put what changed below
the --- line, so I have a hint as to which patch to apply.
Documentation/SubmittingPatches has the full details of how to do this.
Can you resend me the "latest" version of this patch, so I have a chance
of getting it right? :)
thanks,
greg k-h
On Fri, Feb 16, 2018 at 8:32 AM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Mon, Feb 12, 2018 at 05:01:25PM -0800, Joel Fernandes wrote:
>> 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 unlocking the mutex earlier. Functionally that's Ok since
>> we don't need to protect vfs_llseek.
>>
>> [1] https://patchwork.kernel.org/patch/10185031/
>> [2] https://lkml.org/lkml/2018/1/10/48
>>
>> Cc: Todd Kjos <[email protected]>
>> Cc: Arve Hjonnevag <[email protected]>
>> Cc: Greg Hackmann <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: [email protected]
>> Reported-by: [email protected]
>> Signed-off-by: Joel Fernandes <[email protected]>
>> ---
>> drivers/staging/android/ashmem.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> Please always properly version your patches, and put what changed below
> the --- line, so I have a hint as to which patch to apply.
> Documentation/SubmittingPatches has the full details of how to do this.
>
> Can you resend me the "latest" version of this patch, so I have a chance
> of getting it right? :)
Sorry about that :) Fixing now, and will resend. This version you're
replying to is the latest version which is the second version (v2).
- Joel