From patchwork Mon Mar 28 16:59:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Seth Forshee X-Patchwork-Id: 8679941 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 88B109F30C for ; Mon, 28 Mar 2016 17:00:45 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 98CD1201F2 for ; Mon, 28 Mar 2016 17:00:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9D1C9201EC for ; Mon, 28 Mar 2016 17:00:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754916AbcC1RAU (ORCPT ); Mon, 28 Mar 2016 13:00:20 -0400 Received: from mail-ob0-f174.google.com ([209.85.214.174]:36822 "EHLO mail-ob0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754299AbcC1RAR (ORCPT ); Mon, 28 Mar 2016 13:00:17 -0400 Received: by mail-ob0-f174.google.com with SMTP id m7so103794032obh.3 for ; Mon, 28 Mar 2016 10:00:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=WsXkbpvcJfNKxiCiEQrSjd5LiNJz2SUMp85QxLMksIw=; b=GcvLHCDEJYXk9SROojl3sEatV/y8Db1PtaTPLNfBZYCu10rKTpIfnnWqUQ1jW4M6Ah MM/cpn/OOwqTrhEnwOtmB0qqteET5yVmfk/2soYlT1TY2UTmw89yA3vMUlmOQ43nqRpq ceUL6eufPMegFsDQ0nGLDA42dq82VY3f9M6hXKtZCLz39MHuDWXOBefwGmjVM1iBLIxh 2UGkACcXEgj7buFsPj0SFL0wK7l1Rp40vgpHX1Pvc+6Okip8+/k2waaTgOH74IJKln3I oXzWigunwUqGHo5SV/Wm8H9Lb8WbJbTPi4y9NCDNBD+9vbcGOHtsamV95EEvjRWfQGOf ZFuA== 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:references :mime-version:content-disposition:in-reply-to:user-agent; bh=WsXkbpvcJfNKxiCiEQrSjd5LiNJz2SUMp85QxLMksIw=; b=DqUhGMNmNsIiFWvD0hNdI4jesFGGCiVReM8mS8edWVkDiNHkucjPM8QqX8Xr0AIu2z zzvcSsj4L2ICDOsUVH75PU2gNB8SCeNvakKLkQX/ssCMxJKQwbqKFt4dKwRLKOeJJ0C1 PtRA4iT80nY+CmTt7t2QD2XPY7IvPcs/vZnRS25as3yVdltieeuVSN3Ru+DxsnGvRPIC 87S2k3lK7rbzR5KQWNIWNH8mC2SHWrU/0zbRRh9TQOXEyIh6Xa0fMwJ3bW4eqXWMvBls TojFdA1gCHN8szHhpZyESmGMtblKp6wLgFyYeFZ8ga2mZD78lKoDo2kQjou3n41zr2c+ 4PnA== X-Gm-Message-State: AD7BkJIxmGe/yZly62cCdCIN3d3QTp4Zxrji0QVkgOeHkPazI72pOf5JyVptg6cjJEVkmdh7 X-Received: by 10.182.24.8 with SMTP id q8mr12305936obf.67.1459184415978; Mon, 28 Mar 2016 10:00:15 -0700 (PDT) Received: from localhost ([2605:a601:aab:f920:7452:1f6b:5014:d4eb]) by smtp.gmail.com with ESMTPSA id s194sm2493987oie.26.2016.03.28.10.00.14 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 28 Mar 2016 10:00:15 -0700 (PDT) Date: Mon, 28 Mar 2016 11:59:36 -0500 From: Seth Forshee To: "Eric W. Biederman" Cc: Alexander Viro , Serge Hallyn , Richard Weinberger , Austin S Hemmelgarn , Miklos Szeredi , linux-kernel@vger.kernel.org, linux-bcache@vger.kernel.org, dm-devel@redhat.com, linux-raid@vger.kernel.org, linux-mtd@lists.infradead.org, linux-fsdevel@vger.kernel.org, fuse-devel@lists.sourceforge.net, linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov Subject: Re: [PATCH RESEND v2 11/18] fs: Ensure the mounter of a filesystem is privileged towards its inodes Message-ID: <20160328165936.GC137406@ubuntu-hedt> References: <1451930639-94331-1-git-send-email-seth.forshee@canonical.com> <1451930639-94331-12-git-send-email-seth.forshee@canonical.com> <20160303170201.GA30224@ubuntu-hedt> <87twkl50g5.fsf@x220.int.ebiederm.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87twkl50g5.fsf@x220.int.ebiederm.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,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 On Fri, Mar 04, 2016 at 04:43:06PM -0600, Eric W. Biederman wrote: > In general this is only an issue if uids and gids on the filesystem > do not map into the user namespace. > > Therefore the general fix is to limit the logic of checking for > capabilities in s_user_ns if we are dealing with INVALID_UID and > INVALID_GID. For proc and kernfs that should never be the case > so the problem becomes a non-issue. > > Further I would look at limiting that relaxation to just > inode_change_ok. Finally got around to implementing this today; is the patch below what you had in mind? > So that we can easily wrap that check per filesystem > and deny the relaxation for proc and kernfs. proc and kernfs already > have wrappers for .setattr so denying changes when !uid_vaid and > !gid_valid would be a trivial addition, and ensure calamity does > not ensure. I'm confused about this part though. As you say above, proc and kernfs will never have inodes with invalid ids, so it's not an issue. Do you just mean this to be extra insurance against problems? Thanks, Seth --- -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/attr.c b/fs/attr.c index 3cfaaac4a18e..f2bcd3f7dfbb 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -16,6 +16,31 @@ #include #include +static bool chown_ok(const struct inode *inode, kuid_t uid) +{ + if (uid_eq(current_fsuid(), inode->i_uid) && uid_eq(uid, inode->i_uid)) + return true; + if (capable_wrt_inode_uidgid(inode, CAP_CHOWN)) + return true; + if (!uid_valid(inode->i_uid) && + ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN)) + return true; + return false; +} + +static bool chgrp_ok(const struct inode *inode, kgid_t gid) +{ + if (uid_eq(current_fsuid(), inode->i_uid) && + (in_group_p(gid) || gid_eq(gid, inode->i_gid))) + return true; + if (capable_wrt_inode_uidgid(inode, CAP_CHOWN)) + return true; + if (!gid_valid(inode->i_gid) && + ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN)) + return true; + return false; +} + /** * inode_change_ok - check if attribute changes to an inode are allowed * @inode: inode to check @@ -58,17 +83,11 @@ int inode_change_ok(const struct inode *inode, struct iattr *attr) return 0; /* Make sure a caller can chown. */ - if ((ia_valid & ATTR_UID) && - (!uid_eq(current_fsuid(), inode->i_uid) || - !uid_eq(attr->ia_uid, inode->i_uid)) && - !capable_wrt_inode_uidgid(inode, CAP_CHOWN)) + if ((ia_valid & ATTR_UID) && !chown_ok(inode, attr->ia_uid)) return -EPERM; /* Make sure caller can chgrp. */ - if ((ia_valid & ATTR_GID) && - (!uid_eq(current_fsuid(), inode->i_uid) || - (!in_group_p(attr->ia_gid) && !gid_eq(attr->ia_gid, inode->i_gid))) && - !capable_wrt_inode_uidgid(inode, CAP_CHOWN)) + if ((ia_valid & ATTR_GID) && !chgrp_ok(inode, attr->ia_gid)) return -EPERM; /* Make sure a caller can chmod. */