From patchwork Tue Apr 10 19:28:50 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 10333673 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id F158B60365 for ; Tue, 10 Apr 2018 19:31:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E121928112 for ; Tue, 10 Apr 2018 19:31:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D5AE62811E; Tue, 10 Apr 2018 19:31:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5ABA428112 for ; Tue, 10 Apr 2018 19:31:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752564AbeDJTa4 (ORCPT ); Tue, 10 Apr 2018 15:30:56 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:33202 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751611AbeDJTaz (ORCPT ); Tue, 10 Apr 2018 15:30:55 -0400 Received: by mail-pf0-f196.google.com with SMTP id f15so9102463pfn.0; Tue, 10 Apr 2018 12:30:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=/jpGMAgXLPFO34po3CBZB/TMqFZ9l1+lh7zTXmB8CE8=; b=iwFeFlnqFAghg1CDDP0E0UyDi4h22Giikzpf3QRUWDvzZt1kmglpEeBH6r8MCQWbPT VRMbIHoxZyS8EhG52TaaBlt+oXvZlXPUKfGsINeEp7rASg+7GaaQynJHPeKzDPfNOYLu jJ2Q99C1puqHB1wvIEkNwhMNm3yJPqGc7uIhkefhh5e/+kcSUctcj+SrAMb/xtKZFJAD SDcqBiUhYCwl51Iq1oL0sScDRqgGJdT7vphd7JY9UamBKB2sXm67V3jj7BkOFG3inW4y yfhDLyBpEAapqLLpTbtjA+VymBGUcUx2Uz0a2qNB0Qr98jaDfT6FTqQe+IEUsDBErwOg cYaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=/jpGMAgXLPFO34po3CBZB/TMqFZ9l1+lh7zTXmB8CE8=; b=JLkQyjlfwCg2f9r2iwnbZWCS/we35d4ZcKmYX356iLBLGLUULH7XyglDGZOkDXJXR4 KOjatFZFftrm5h8TvVwtkm2RQs+vOfP4lE76KjkWgSA2sySZQXlBSVY25GQt9UEmpl0g rS5gqaGkkBG3JJsLTCnCCgzWnpULmX3ef6t9X07Y1RRUrAoa1co3PITcSgi+KeMXcorJ DgFolfROA7kGxSfDu55BEkTCbqTfGnI213WwIo5IVU+I8ccWI8tuuroiPbP2RG4pihYB MNGoIUYPR3ptpLtx1mE8+sW2M3KxXH7wjlSkOFuZIFjwbK4HEX5SJaslW7abYUz9eGxR dqGQ== X-Gm-Message-State: ALQs6tDP7k1GKbYA+JFKlSFoiH3Vsf8ZYfcv6r4QT05vHLTHtYSAukpi 4f3JAen3AI04/lCkjX6eOt3/hBQL X-Google-Smtp-Source: AIpwx4/pTwja7oduonka5xw1CEoFbanuHw8uKAREaoBTsMbBlldSVqf1QcRDQc7il8avK+dyohhShw== X-Received: by 10.99.138.202 with SMTP id y193mr1200182pgd.224.1523388654182; Tue, 10 Apr 2018 12:30:54 -0700 (PDT) Received: from ebiggers-linuxstation.kir.corp.google.com ([2620:15c:17:3:dc28:5c82:b905:e8a8]) by smtp.gmail.com with ESMTPSA id v8sm6166359pff.42.2018.04.10.12.30.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Apr 2018 12:30:53 -0700 (PDT) From: Eric Biggers To: linux-mm@kvack.org, Andrew Morton Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, "Kirill A . Shutemov" , Davidlohr Bueso , Manfred Spraul , "Eric W . Biederman" , syzkaller-bugs@googlegroups.com Subject: [PATCH v2] ipc/shm: fix use-after-free of shm file via remap_file_pages() Date: Tue, 10 Apr 2018 12:28:50 -0700 Message-Id: <20180410192850.235835-1-ebiggers3@gmail.com> X-Mailer: git-send-email 2.17.0.484.g0c8726318c-goog In-Reply-To: <20180410191413.GA214391@gmail.com> References: <20180410191413.GA214391@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 I think caused it. When remap_file_pages() is passed a full System V shared memory segment, the memory is first unmapped, then a new map is created using the ->vm_file. Between these steps, the shm ID can be removed and reused for a new shm segment. But, shm_mmap() only checks whether the ID is currently valid before calling the underlying file's ->mmap(); it doesn't check whether it was reused. Thus it can use the wrong underlying file, one that was already freed. Fix this by making the "outer" shm file (the one that gets put in ->vm_file) hold a reference to the real shm file, and by making __shm_open() require that the file associated with the shm ID matches the one associated with the "outer" file. Taking the reference to the real shm file is needed to fully solve the problem, since otherwise sfd->file could point to a freed file, which then could be reallocated for the reused shm ID, causing the wrong shm segment to be mapped (and without the required permission checks). Commit 1ac0b6dec656 ("ipc/shm: handle removed segments gracefully in shm_mmap()") almost fixed this bug, but it didn't go far enough because it didn't consider the case where the shm ID is reused. The following program usually reproduces this bug: #include #include #include #include int main() { int is_parent = (fork() != 0); srand(getpid()); for (;;) { int id = shmget(0xF00F, 4096, IPC_CREAT|0700); if (is_parent) { void *addr = shmat(id, NULL, 0); usleep(rand() % 50); while (!syscall(__NR_remap_file_pages, addr, 4096, 0, 0, 0)); } else { usleep(rand() % 50); shmctl(id, IPC_RMID, NULL); } } } It causes the following NULL pointer dereference due to a 'struct file' being used while it's being freed. (I couldn't actually get a KASAN use-after-free splat like in the syzbot report. But I think it's possible with this bug; it would just take a more extraordinary race...) BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 PGD 0 P4D 0 Oops: 0000 [#1] SMP NOPTI CPU: 9 PID: 258 Comm: syz_ipc Not tainted 4.16.0-05140-gf8cf2f16a7c95 #189 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014 RIP: 0010:d_inode include/linux/dcache.h:519 [inline] RIP: 0010:touch_atime+0x25/0xd0 fs/inode.c:1724 [...] Call Trace: file_accessed include/linux/fs.h:2063 [inline] shmem_mmap+0x25/0x40 mm/shmem.c:2149 call_mmap include/linux/fs.h:1789 [inline] shm_mmap+0x34/0x80 ipc/shm.c:465 call_mmap include/linux/fs.h:1789 [inline] mmap_region+0x309/0x5b0 mm/mmap.c:1712 do_mmap+0x294/0x4a0 mm/mmap.c:1483 do_mmap_pgoff include/linux/mm.h:2235 [inline] SYSC_remap_file_pages mm/mmap.c:2853 [inline] SyS_remap_file_pages+0x232/0x310 mm/mmap.c:2769 do_syscall_64+0x64/0x1a0 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x42/0xb7 Reported-by: syzbot+d11f321e7f1923157eac80aa990b446596f46439@syzkaller.appspotmail.com Fixes: c8d78c1823f4 ("mm: replace remap_file_pages() syscall with emulation") Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers --- ipc/shm.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) v2: update commit message and add comment to explain why we need to take a reference to the real shm file. diff --git a/ipc/shm.c b/ipc/shm.c index acefe44fefef..f06505c68cc9 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -225,6 +225,12 @@ static int __shm_open(struct vm_area_struct *vma) if (IS_ERR(shp)) return PTR_ERR(shp); + if (shp->shm_file != sfd->file) { + /* ID was reused */ + shm_unlock(shp); + return -EINVAL; + } + shp->shm_atim = ktime_get_real_seconds(); ipc_update_pid(&shp->shm_lprid, task_tgid(current)); shp->shm_nattch++; @@ -455,8 +461,9 @@ static int shm_mmap(struct file *file, struct vm_area_struct *vma) int ret; /* - * In case of remap_file_pages() emulation, the file can represent - * removed IPC ID: propogate shm_lock() error to caller. + * In case of remap_file_pages() emulation, the file can represent an + * IPC ID that was removed, and possibly even reused by another shm + * segment already. Propagate this case as an error to caller. */ ret = __shm_open(vma); if (ret) @@ -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); return 0; @@ -1432,7 +1440,16 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, file->f_mapping = shp->shm_file->f_mapping; sfd->id = shp->shm_perm.id; sfd->ns = get_ipc_ns(ns); - sfd->file = shp->shm_file; + /* + * We need to take a reference to the real shm file to prevent the + * pointer from becoming stale in cases where the lifetime of the outer + * file extends beyond that of the shm segment. It's not usually + * possible, but it can happen during remap_file_pages() emulation as + * that unmaps the memory, then does ->mmap() via file reference only. + * We'll deny the ->mmap() if the shm segment was since removed, but to + * detect shm ID reuse we need to compare the file pointers. + */ + sfd->file = get_file(shp->shm_file); sfd->vm_ops = NULL; err = security_mmap_file(file, prot, flags);