From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by kanga.kvack.org (Postfix) with ESMTP id 0F2FE6B0003 for ; Mon, 21 Dec 2015 10:44:55 -0500 (EST) Received: by mail-wm0-f48.google.com with SMTP id l126so73895549wml.1 for ; Mon, 21 Dec 2015 07:44:55 -0800 (PST) Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com. [2a00:1450:400c:c09::22e]) by mx.google.com with ESMTPS id k7si50013546wjz.162.2015.12.21.07.44.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Dec 2015 07:44:54 -0800 (PST) Received: by mail-wm0-x22e.google.com with SMTP id p187so73838998wmp.1 for ; Mon, 21 Dec 2015 07:44:53 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20151105142336.46D907FD@black.fi.intel.com> References: <20151012122702.GC2544@node> <20151012174945.GC3170@linux-uzut.site> <20151012181040.GC6447@node> <20151012185533.GD3170@linux-uzut.site> <20151013031821.GA3052@linux-uzut.site> <20151013123028.GA12934@node> <20151105142336.46D907FD@black.fi.intel.com> From: Dmitry Vyukov Date: Mon, 21 Dec 2015 16:44:34 +0100 Message-ID: Subject: Re: GPF in shm_lock ipc Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: To: syzkaller Cc: "Kirill A. Shutemov" , Andrew Morton , Dave Hansen , Hugh Dickins , Joe Perches , sds@tycho.nsa.gov, Oleg Nesterov , "Kirill A. Shutemov" , Rik van Riel , mhocko@suse.cz, gang.chen.5i5j@gmail.com, Peter Feiner , Andrea Arcangeli , "linux-mm@kvack.org" , LKML , Kostya Serebryany , Alexander Potapenko , Andrey Konovalov , Sasha Levin , Manfred Spraul On Thu, Nov 5, 2015 at 3:23 PM, Kirill A. Shutemov wrote: > What about this: Ping. This is still happening for me on tip. Can we pull in this fix if it looks good to everybody? > From 06b0fc9d62592f6f3ad9f45cccf1f6a5b3113bdc Mon Sep 17 00:00:00 2001 > From: "Kirill A. Shutemov" > Date: Thu, 5 Nov 2015 15:53:04 +0200 > Subject: [PATCH] ipc/shm: handle removed segments gracefully in shm_mmap() > > 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 > #include > #include > #include > > #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 > > Signed-off-by: Kirill A. Shutemov > Reported-by: Dmitry Vyukov > Cc: Davidlohr Bueso > --- > ipc/shm.c | 53 +++++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 43 insertions(+), 10 deletions(-) > > diff --git a/ipc/shm.c b/ipc/shm.c > index 41787276e141..3174634ca4e5 100644 > --- a/ipc/shm.c > +++ b/ipc/shm.c > @@ -156,11 +156,12 @@ static inline struct shmid_kernel *shm_lock(struct ipc_namespace *ns, int id) > struct kern_ipc_perm *ipcp = ipc_lock(&shm_ids(ns), id); > > /* > - * We raced in the idr lookup or with shm_destroy(). Either way, the > - * ID is busted. > + * Callers of shm_lock() must validate the status of the returned ipc > + * object pointer (as returned by ipc_lock()), and error out as > + * appropriate. > */ > - WARN_ON(IS_ERR(ipcp)); > - > + if (IS_ERR(ipcp)) > + return (void *)ipcp; > return container_of(ipcp, struct shmid_kernel, shm_perm); > } > > @@ -186,18 +187,33 @@ static inline void shm_rmid(struct ipc_namespace *ns, struct shmid_kernel *s) > } > > > -/* This is called by fork, once for every shm attach. */ > -static void shm_open(struct vm_area_struct *vma) > +static int __shm_open(struct vm_area_struct *vma) > { > struct file *file = vma->vm_file; > struct shm_file_data *sfd = shm_file_data(file); > struct shmid_kernel *shp; > > shp = shm_lock(sfd->ns, sfd->id); > + > + if (IS_ERR(shp)) > + return PTR_ERR(shp); > + > shp->shm_atim = get_seconds(); > shp->shm_lprid = task_tgid_vnr(current); > shp->shm_nattch++; > shm_unlock(shp); > + return 0; > +} > + > +/* This is called by fork, once for every shm attach. */ > +static void shm_open(struct vm_area_struct *vma) > +{ > + int err = __shm_open(vma); > + /* > + * We raced in the idr lookup or with shm_destroy(). > + * Either way, the ID is busted. > + */ > + WARN_ON_ONCE(err); > } > > /* > @@ -260,6 +276,14 @@ static void shm_close(struct vm_area_struct *vma) > down_write(&shm_ids(ns).rwsem); > /* remove from the list of attaches of the shm segment */ > shp = shm_lock(ns, sfd->id); > + > + /* > + * We raced in the idr lookup or with shm_destroy(). > + * Either way, the ID is busted. > + */ > + if (WARN_ON_ONCE(IS_ERR(shp))) > + goto done; /* no-op */ > + > shp->shm_lprid = task_tgid_vnr(current); > shp->shm_dtim = get_seconds(); > shp->shm_nattch--; > @@ -267,6 +291,7 @@ static void shm_close(struct vm_area_struct *vma) > shm_destroy(ns, shp); > else > shm_unlock(shp); > +done: > up_write(&shm_ids(ns).rwsem); > } > > @@ -388,17 +413,25 @@ static int shm_mmap(struct file *file, struct vm_area_struct *vma) > struct shm_file_data *sfd = shm_file_data(file); > int ret; > > + /* > + * In case of remap_file_pages() emulation, the file can represent > + * removed IPC ID: propogate shm_lock() error to caller. > + */ > + ret =__shm_open(vma); > + if (ret) > + return ret; > + > ret = sfd->file->f_op->mmap(sfd->file, vma); > - if (ret != 0) > + if (ret) { > + shm_close(vma); > return ret; > + } > sfd->vm_ops = vma->vm_ops; > #ifdef CONFIG_MMU > WARN_ON(!sfd->vm_ops->fault); > #endif > vma->vm_ops = &shm_vm_ops; > - shm_open(vma); > - > - return ret; > + return 0; > } > > static int shm_release(struct inode *ino, struct file *file) -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org