staging: android: ashmem: Fix lockdep warning for write operation
Subsystems (2)
PATCH 1/1 staging: android: ashmem: Fix lockdep warning for write operation
syzbot report [1] describes a deadlock when write operation against an ashmem fd executed at the time when ashmem is shrinking its cache results in the following lock sequence:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#13);
lock(fs_reclaim); lock(&sb->s_type->i_mutex_key#13);
kswapd takes fs_reclaim and then inode_lock while generic_perform_write takes inode_lock and then fs_reclaim. However ashmem does not support writing into backing shmem with a write syscall. The only way to change its content is to mmap it and operate on mapped memory. Therefore the race that lockdep is warning about is not valid. Resolve this by introducing a separate lockdep class for the backing shmem inodes.
[1]: https://lkml.kernel.org/lkml/[email protected]/
--- drivers/staging/android/ashmem.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/staging/android/ashmem.c +12 -0
@@ -95,6 +95,15 @@ static DEFINE_MUTEX(ashmem_mutex);static struct kmem_cache *ashmem_area_cachep__read_mostly ;static struct kmem_cache *ashmem_range_cachep__read_mostly ; +/* +* A separate lockdep class for the backing shmem inodes to resolve the lockdep +* warning about the race between kswapd taking fs_reclaim before inode_lock +* and write syscall taking inode_lock and then fs_reclaim. +* Note that such race is impossible because ashmem does not support write +* syscalls operating on the backing shmem. +*/ +static struct lock_class_key backing_shmem_inode_class; +static inline unsigned long range_size (struct ashmem_range *range) {return range->pgend - range->pgstart +1 ;
@@ -396,6 +405,7 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)if (!asma->file) {char *name =ASHMEM_NAME_DEF ;struct file *vmfile; +struct inode *inode;if (asma->name[ASHMEM_NAME_PREFIX_LEN ] !='\0' ) name = asma->name;
@@ -407,6 +417,8 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) goto out; } vmfile->f_mode |=FMODE_LSEEK ; + inode =file_inode (vmfile); +lockdep_set_class (&inode->i_rwsem, &backing_shmem_inode_class); asma->file = vmfile;/* * override mmap operation of the vmfile so that it can't be
Signature · 1 line
2.28.0.rc0.105.gf9edc3c819-goog
Quoted 20 lines · Suren Baghdasaryan
syzbot report [1] describes a deadlock when write operation against an ashmem fd executed at the time when ashmem is shrinking its cache results in the following lock sequence:
Possible unsafe locking scenario:CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&sb->s_type->i_mutex_key#13); lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#13);kswapd takes
fs_reclaimand theninode_lockwhilegeneric_perform_writetakesinode_lockand thenfs_reclaim. However ashmem does not support writing into backing shmem with a write syscall. The only way to change its content is to mmap it and operate on mapped memory. Therefore the race that lockdep is warning about is not valid. Resolve this by introducing a separate lockdep class for the backing shmem inodes.
Quoted 3 lines
[1]: https://lkml.kernel.org/lkml/[email protected]/
Signed-off-by Suren Baghdasaryan <[email protected]>
Please add proper tags:
The Reported-by tag to use was given in the original syzbot report.
- Eric
Quoted 26 lines · Eric Biggers
Quoted 20 lines · Suren Baghdasaryan
syzbot report [1] describes a deadlock when write operation against an ashmem fd executed at the time when ashmem is shrinking its cache results in the following lock sequence:
Possible unsafe locking scenario:CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&sb->s_type->i_mutex_key#13); lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#13);kswapd takes
fs_reclaimand theninode_lockwhilegeneric_perform_writetakesinode_lockand thenfs_reclaim. However ashmem does not support writing into backing shmem with a write syscall. The only way to change its content is to mmap it and operate on mapped memory. Therefore the race that lockdep is warning about is not valid. Resolve this by introducing a separate lockdep class for the backing shmem inodes.Quoted 3 lines
[1]: https://lkml.kernel.org/lkml/[email protected]/
Signed-off-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
Please add proper tags:
The Reported-by tag to use was given in the original syzbot report.
Will add in v2. Thanks!
Quoted 2 lines
- Eric
Quoted 21 lines · Suren Baghdasaryan
syzbot report [1] describes a deadlock when write operation against an ashmem fd executed at the time when ashmem is shrinking its cache results in the following lock sequence:
Possible unsafe locking scenario:CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&sb->s_type->i_mutex_key#13); lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#13);kswapd takes
fs_reclaimand theninode_lockwhilegeneric_perform_writetakesinode_lockand thenfs_reclaim. However ashmem does not support writing into backing shmem with a write syscall. The only way to change its content is to mmap it and operate on mapped memory. Therefore the race that lockdep is warning about is not valid. Resolve this by introducing a separate lockdep class for the backing shmem inodes.
Quoted 4 lines
[1]: https://lkml.kernel.org/lkml/[email protected]/
Signed-off-by Suren Baghdasaryan <[email protected]>---
Once Eric's nits are resolved:
Thanks.
Quoted 23 lines · Joel Fernandes
Quoted 21 lines · Suren Baghdasaryan
syzbot report [1] describes a deadlock when write operation against an ashmem fd executed at the time when ashmem is shrinking its cache results in the following lock sequence:
Possible unsafe locking scenario:CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&sb->s_type->i_mutex_key#13); lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#13);kswapd takes
fs_reclaimand theninode_lockwhilegeneric_perform_writetakesinode_lockand thenfs_reclaim. However ashmem does not support writing into backing shmem with a write syscall. The only way to change its content is to mmap it and operate on mapped memory. Therefore the race that lockdep is warning about is not valid. Resolve this by introducing a separate lockdep class for the backing shmem inodes.
Quoted 8 lines
Quoted 4 lines
[1]: https://lkml.kernel.org/lkml/[email protected]/
Signed-off-by Suren Baghdasaryan <[email protected]>---
Once Eric's nits are resolved:
Reviewed-by Joel Fernandes (Google) <[email protected]>
Thanks Joel!
I'm fixing the nits and will report the patch shortly. One note about adding the "Fixes: " tag - this is a fix for a false positive lockdep warning and it's unclear which patch should be quoted here (I could not find a clear cause that started this warning). In similar situations, for example here: https://lkml.org/lkml/2020/6/15/958
developers seem to skip that tag. So I'll do the same.
Quoted 2 lines
Thanks.