Messages in this thread | | | Date | Wed, 11 Nov 2015 09:03:47 -0800 | From | Davidlohr Bueso <> | Subject | Re: [PATCH, RESEND] ipc/shm: handle removed segments gracefully in shm_mmap() |
| |
On Wed, 11 Nov 2015, Kirill A. Shutemov wrote:
>remap_file_pages(2) emulation can reach file which represents removed >IPC ID as long as a memory segment is mapped. It breaks expectations >of IPC subsystem. > >Test case (rewritten to be more human readable, originally autogenerated >by syzkaller[1]): > > #define _GNU_SOURCE > #include <stdlib.h> > #include <sys/ipc.h> > #include <sys/mman.h> > #include <sys/shm.h> > > #define PAGE_SIZE 4096 > > int main() > { > int id; > void *p; > > id = shmget(IPC_PRIVATE, 3 * PAGE_SIZE, 0); > p = shmat(id, NULL, 0); > shmctl(id, IPC_RMID, NULL); > remap_file_pages(p, 3 * PAGE_SIZE, 0, 7, 0); > > return 0; > } > >The patch changes shm_mmap() and code around shm_lock() to propagate >locking error back to caller of shm_mmap(). > >[1] http://github.com/google/syzkaller
So this is a very similar approach that I posted back when this discussion arose: https://lkml.org/lkml/2015/10/12/959 -- There are a few differences for which I prefer mine :)
o My shm_check_vma_validity() also deals with IPC_RMID as we do the ipc_valid_object() check.
o We have a new WARN where necessary, instead of having one now is shm_open.
o My no-ops explicitly pair.
[...]
> ret = sfd->file->f_op->mmap(sfd->file, vma); >- if (ret != 0) >+ if (ret) { >+ shm_close(vma); > return ret; >+ }
Hmm what's this shm_close() about?
Thanks, Davidlohr
| |