Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-10 Thread Eric Biggers
On Tue, Apr 10, 2018 at 10:58:22AM +0300, Kirill A. Shutemov wrote: > On Mon, Apr 09, 2018 at 01:36:35PM -0700, Eric Biggers wrote: > > On Mon, Apr 09, 2018 at 01:12:32PM -0700, Davidlohr Bueso wrote: > > > On Mon, 09 Apr 2018, Eric Biggers wrote: > > > > > > > It's necessary because if we don't

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-10 Thread Eric Biggers
On Tue, Apr 10, 2018 at 10:58:22AM +0300, Kirill A. Shutemov wrote: > On Mon, Apr 09, 2018 at 01:36:35PM -0700, Eric Biggers wrote: > > On Mon, Apr 09, 2018 at 01:12:32PM -0700, Davidlohr Bueso wrote: > > > On Mon, 09 Apr 2018, Eric Biggers wrote: > > > > > > > It's necessary because if we don't

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-10 Thread Davidlohr Bueso
On Sun, 08 Apr 2018, Eric Biggers wrote: @@ -480,6 +487,7 @@ static int shm_release(struct inode *ino, struct file *file) struct shm_file_data *sfd = shm_file_data(file); put_ipc_ns(sfd->ns); + fput(sfd->file); shm_file_data(file) = NULL; kfree(sfd);

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-10 Thread Davidlohr Bueso
On Sun, 08 Apr 2018, Eric Biggers wrote: @@ -480,6 +487,7 @@ static int shm_release(struct inode *ino, struct file *file) struct shm_file_data *sfd = shm_file_data(file); put_ipc_ns(sfd->ns); + fput(sfd->file); shm_file_data(file) = NULL; kfree(sfd);

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-10 Thread Kirill A. Shutemov
On Mon, Apr 09, 2018 at 01:36:35PM -0700, Eric Biggers wrote: > On Mon, Apr 09, 2018 at 01:12:32PM -0700, Davidlohr Bueso wrote: > > On Mon, 09 Apr 2018, Eric Biggers wrote: > > > > > It's necessary because if we don't hold a reference to sfd->file, then it > > > can be > > > a stale pointer

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-10 Thread Kirill A. Shutemov
On Mon, Apr 09, 2018 at 01:36:35PM -0700, Eric Biggers wrote: > On Mon, Apr 09, 2018 at 01:12:32PM -0700, Davidlohr Bueso wrote: > > On Mon, 09 Apr 2018, Eric Biggers wrote: > > > > > It's necessary because if we don't hold a reference to sfd->file, then it > > > can be > > > a stale pointer

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Davidlohr Bueso
On Mon, 09 Apr 2018, Davidlohr Bueso wrote: So I don't think the pointer is going anywhere, or am I missing something? Ah, yes, wrong pointer, this is sdf->file -- sorry for the noise.

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Davidlohr Bueso
On Mon, 09 Apr 2018, Davidlohr Bueso wrote: So I don't think the pointer is going anywhere, or am I missing something? Ah, yes, wrong pointer, this is sdf->file -- sorry for the noise.

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Eric Biggers
On Mon, Apr 09, 2018 at 01:12:32PM -0700, Davidlohr Bueso wrote: > On Mon, 09 Apr 2018, Eric Biggers wrote: > > > It's necessary because if we don't hold a reference to sfd->file, then it > > can be > > a stale pointer when we compare it in __shm_open(). In particular, if the > > new > >

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Eric Biggers
On Mon, Apr 09, 2018 at 01:12:32PM -0700, Davidlohr Bueso wrote: > On Mon, 09 Apr 2018, Eric Biggers wrote: > > > It's necessary because if we don't hold a reference to sfd->file, then it > > can be > > a stale pointer when we compare it in __shm_open(). In particular, if the > > new > >

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Davidlohr Bueso
On Mon, 09 Apr 2018, Eric Biggers wrote: It's necessary because if we don't hold a reference to sfd->file, then it can be a stale pointer when we compare it in __shm_open(). In particular, if the new struct file happened to be allocated at the same address as the old one, then 'sfd->file ==

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Davidlohr Bueso
On Mon, 09 Apr 2018, Eric Biggers wrote: It's necessary because if we don't hold a reference to sfd->file, then it can be a stale pointer when we compare it in __shm_open(). In particular, if the new struct file happened to be allocated at the same address as the old one, then 'sfd->file ==

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Eric Biggers
On Mon, Apr 09, 2018 at 12:48:14PM +0300, Kirill A. Shutemov wrote: > On Mon, Apr 09, 2018 at 04:30:39AM +, Eric Biggers wrote: > > diff --git a/ipc/shm.c b/ipc/shm.c > > index acefe44fefefa..c80c5691a9970 100644 > > --- a/ipc/shm.c > > +++ b/ipc/shm.c > > @@ -225,6 +225,12 @@ static int

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Eric Biggers
On Mon, Apr 09, 2018 at 12:48:14PM +0300, Kirill A. Shutemov wrote: > On Mon, Apr 09, 2018 at 04:30:39AM +, Eric Biggers wrote: > > diff --git a/ipc/shm.c b/ipc/shm.c > > index acefe44fefefa..c80c5691a9970 100644 > > --- a/ipc/shm.c > > +++ b/ipc/shm.c > > @@ -225,6 +225,12 @@ static int

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Kirill A. Shutemov
On Mon, Apr 09, 2018 at 04:30:39AM +, Eric Biggers wrote: > From: Eric Biggers > > syzbot reported a use-after-free of shm_file_data(file)->file->f_op in > shm_get_unmapped_area(), called via sys_remap_file_pages(). > Unfortunately it couldn't generate a reproducer, but

Re: [PATCH] ipc/shm: fix use-after-free of shm file via remap_file_pages()

2018-04-09 Thread Kirill A. Shutemov
On Mon, Apr 09, 2018 at 04:30:39AM +, Eric Biggers wrote: > From: Eric Biggers > > syzbot reported a use-after-free of shm_file_data(file)->file->f_op in > shm_get_unmapped_area(), called via sys_remap_file_pages(). > Unfortunately it couldn't generate a reproducer, but I found a bug which >