| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 Dec 2015 16:44:34 +0100
From: Dmitry Vyukov <dvyukov@...gle.com>
To: syzkaller <syzkaller@...glegroups.com>
Cc: "Kirill A. Shutemov" <kirill@...temov.name>,
Andrew Morton <akpm@...ux-foundation.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Hugh Dickins <hughd@...gle.com>, Joe Perches <joe@...ches.com>,
sds@...ho.nsa.gov, Oleg Nesterov <oleg@...hat.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Rik van Riel <riel@...hat.com>, mhocko@...e.cz,
gang.chen.5i5j@...il.com, Peter Feiner <pfeiner@...gle.com>,
Andrea Arcangeli <aarcange@...hat.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Andrey Konovalov <andreyknvl@...gle.com>,
Sasha Levin <sasha.levin@...cle.com>,
Manfred Spraul <manfred@...orfullife.com>
Subject: Re: GPF in shm_lock ipc
On Thu, Nov 5, 2015 at 3:23 PM, Kirill A. Shutemov
<kirill.shutemov@...ux.intel.com> 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" <kirill.shutemov@...ux.intel.com>
> 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 <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
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: Davidlohr Bueso <dave@...olabs.net>
> ---
> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists