From patchwork Wed Dec 9 22:51:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 7813071 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id D78AABEEE1 for ; Wed, 9 Dec 2015 22:58:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C1AC32054A for ; Wed, 9 Dec 2015 22:58:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C4DAA2053E for ; Wed, 9 Dec 2015 22:58:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754192AbbLIW6o (ORCPT ); Wed, 9 Dec 2015 17:58:44 -0500 Received: from mail-pf0-f180.google.com ([209.85.192.180]:34992 "EHLO mail-pf0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753550AbbLIW6n (ORCPT ); Wed, 9 Dec 2015 17:58:43 -0500 Received: by pfu207 with SMTP id 207so37075598pfu.2 for ; Wed, 09 Dec 2015 14:58:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition; bh=YjKLXjFjTAp5ryoNUucFyxzGzJwG368ujbFda2RsVtc=; b=djM4YnYykxGyOAI8kBSAC5pk3M57qnfUHGvhJPNZly/TnZLF4d8lViuH0xFxXk4nWc cuVCZczgZdJY6IEVkNBFqB1b4YQ4qLxGhqsmdbzT5otc9oC6HGZYYVqSptk6NumQZ8iG QGHRaSDfo6lansVG6tJZMhIG6MYwg17G8eFbQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-type:content-disposition; bh=YjKLXjFjTAp5ryoNUucFyxzGzJwG368ujbFda2RsVtc=; b=IhbbukEBN/4Nt9+d6uCcwiyNXbsLYy/wgWXi8C2Xz2RWsk2JlP9MQPL7PBUSVx5zQU TWRjWaK7lSCEVlzMhf4q0wWlZXOUc6kez9a4UCBDvUDwm9b6lBZQD1t2rzPtoYARXqZD tbiUIkOoC/nMKMXAbreAOkenPDpikjGxotwo5FDkS7osJULkA8uaXrtCLB9t3TwPi8uL Vp3BTmERTMFGHCFgHPtQmlr4tPid76nzl583oDLu9qgQZ5bokTzxpv98sogdkLOlEkZp 7mKtTFGZOQve/5569Evw7YAEuwo0wmHLQabM+CMomtEXfKm3iOUwJq+eIh4VTSreJyXQ l80g== X-Gm-Message-State: ALoCoQkabtWUo2dvsN6B/LWfENkG64gDz8APppFDKneUnHYjAW8VUlA6kwD8uFu4spSdhhjuz1vEszEppvN0S2OXbFFF76s2Sw== X-Received: by 10.98.16.81 with SMTP id y78mr2051354pfi.158.1449701510624; Wed, 09 Dec 2015 14:51:50 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id n16sm13897762pfa.53.2015.12.09.14.51.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Dec 2015 14:51:50 -0800 (PST) Date: Wed, 9 Dec 2015 14:51:48 -0800 From: Kees Cook To: Andrew Morton Cc: Jan Kara , yalin wang , Willy Tarreau , "Eric W. Biederman" , Alexander Viro , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH v5] fs: clear file privilege bits when mmap writing Message-ID: <20151209225148.GA14794@www.outflux.net> MIME-Version: 1.0 Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Normally, when a user can modify a file that has setuid or setgid bits, those bits are cleared when they are not the file owner or a member of the group. This is enforced when using write and truncate but not when writing to a shared mmap on the file. This could allow the file writer to gain privileges by changing a binary without losing the setuid/setgid/caps bits. Changing the bits requires holding inode->i_mutex, so it cannot be done during the page fault (due to mmap_sem being held during the fault). We could do this during vm_mmap_pgoff, but that would need coverage in mprotect as well, but to check for MAP_SHARED, we'd need to hold mmap_sem again. We could clear at open() time, but it's possible things are accidentally opening with O_RDWR and only reading. Better to clear on close and error failures (i.e. an improvement over now, which is not clearing at all). Instead, detect the need to clear the bits during the page fault, and actually remove the bits during final fput. Since the file was open for writing, it wouldn't have been possible to execute it yet. Signed-off-by: Kees Cook --- I think this is the best we can do; everything else is blocked by mmap_sem. --- fs/file_table.c | 11 +++++++++++ include/linux/fs.h | 1 + mm/memory.c | 1 + 3 files changed, 13 insertions(+) diff --git a/fs/file_table.c b/fs/file_table.c index ad17e05ebf95..3a7eee76ea90 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -191,6 +191,17 @@ static void __fput(struct file *file) might_sleep(); + /* + * XXX: While avoiding mmap_sem, we've already been written to. + * We must ignore the return value, since we can't reject the + * write. + */ + if (unlikely(file->f_remove_privs)) { + mutex_lock(&inode->i_mutex); + file_remove_privs(file); + mutex_unlock(&inode->i_mutex); + } + fsnotify_close(file); /* * The function eventpoll_release() should be the first called diff --git a/include/linux/fs.h b/include/linux/fs.h index 3aa514254161..409bd7047e7e 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -872,6 +872,7 @@ struct file { struct list_head f_tfile_llink; #endif /* #ifdef CONFIG_EPOLL */ struct address_space *f_mapping; + bool f_remove_privs; } __attribute__((aligned(4))); /* lest something weird decides that 2 is OK */ struct file_handle { diff --git a/mm/memory.c b/mm/memory.c index c387430f06c3..08a77e0cf65f 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2036,6 +2036,7 @@ static inline int wp_page_reuse(struct mm_struct *mm, if (!page_mkwrite) file_update_time(vma->vm_file); + vma->vm_file->f_remove_privs = true; } return VM_FAULT_WRITE;