From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AF2C7C169C4 for ; Thu, 31 Jan 2019 10:21:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 87AE0218AF for ; Thu, 31 Jan 2019 10:21:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731625AbfAaKVJ (ORCPT ); Thu, 31 Jan 2019 05:21:09 -0500 Received: from mail-oi1-f196.google.com ([209.85.167.196]:43749 "EHLO mail-oi1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726368AbfAaKVI (ORCPT ); Thu, 31 Jan 2019 05:21:08 -0500 Received: by mail-oi1-f196.google.com with SMTP id u18so2244954oie.10 for ; Thu, 31 Jan 2019 02:21:08 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=m0SuNou7yDS+3D1wYaKFTHEBe7Ne+CO8BHGdPi4i8lg=; b=k3x3RQV347uKzc4ha6tb6bTYv7cDH71eZpxrrFqalKIJ1/Fwk1wyOEwe7p5/TB3udA +pfLIDKZXHUyDQNH1lVZk1u7TuirE4sXzJZAuK3TxN+PG9epvfU9SmOo3RYFd9S12+DR 0U0NP0mf9Jq+dnGcJRBQjArXNiPNzlFOgx1HUkXDcc66YtjF5b4R/WcWGfIIge3J4LnB CG52JpH02+PNJfzJqOJYnxWgGRp6NkltL8KT/QZ+pa1gYDuc2gT89JQZV1cBZyGHmpwg kS3L14z9h2/ad13rsEz1aLG7KETV6mmWL614U7li8E2oFgeK0MzjFldjc7tjgs/iB4Rr 2WcA== X-Gm-Message-State: AHQUAua2VqdtZJL4pjyMr+B8UBKhVLz8Kn3nSAWNI/Tohe/SIJxJ9XOQ iap3NDYcGxggxLQGvdB5ljb+D69AQV0o0OpE9gGmcw== X-Google-Smtp-Source: AHgI3IbC5A8FjeSM6t+zqElCrlOi/AmHx+JTKGjcCe/AGqytjqX+Jci3QX3z+Let7pLVO/WyG1NRGykj83KC1lYAL4g= X-Received: by 2002:aca:2403:: with SMTP id n3mr2857487oic.328.1548930068092; Thu, 31 Jan 2019 02:21:08 -0800 (PST) MIME-Version: 1.0 References: <20190130114150.27807-1-omosnace@redhat.com> <20190130114150.27807-6-omosnace@redhat.com> <20190130170911.GZ50184@devbig004.ftw2.facebook.com> In-Reply-To: <20190130170911.GZ50184@devbig004.ftw2.facebook.com> From: Ondrej Mosnacek Date: Thu, 31 Jan 2019 11:20:57 +0100 Message-ID: Subject: Re: [PATCH v3 5/5] kernfs: initialize security of newly created nodes To: Tejun Heo Cc: selinux@vger.kernel.org, Paul Moore , Stephen Smalley , Linux Security Module list , Casey Schaufler , Greg Kroah-Hartman , linux-fsdevel@vger.kernel.org, cgroups@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Hi Tejun, On Wed, Jan 30, 2019 at 6:09 PM Tejun Heo wrote: > > Hello, > > On Wed, Jan 30, 2019 at 12:41:50PM +0100, Ondrej Mosnacek wrote: > > @@ -673,6 +698,12 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root, > > goto err_out3; > > } > > > > + if (parent) { > > + ret = kernfs_node_init_security(parent, kn); > > + if (ret) > > + goto err_out3; > > + } > > So, doing this unconditionally isn't a good idea. kernfs doesn't use > the usual dentry/inode because there are machines with 6, even 7 digit > number of kernfs nodes and some of them even failed to boot due to > memory shortage. Please don't blow it up by default. Hm, I see... basically the only thing that gets allocated in kernfs_node_init_security() by default (at least under SELinux/ no LSM) is the kernfs_iattrs structures, so I assume you are pointing at that. I think this can be easily fixed, if we again use the assumption that whenever the parent node has only default attributes (parent->iattrs == NULL), then the child node should also have just default attributes (and so we don't need to call kernfs_iattrs() on it nor call the security hook). Something along these lines: [...] +static int kernfs_node_init_security(struct kernfs_node *parent, + struct kernfs_node *kn) +{ + struct kernfs_iattrs *attrs, *pattrs; + struct qstr q; + + pattrs = parent->iattrs; + if (!pattrs) + return 0; + + attrs = kernfs_iattrs(kn); + if (!attrs) + return -ENOMEM; + + q.name = kn->name; + q.hash_len = hashlen_string(parent, kn->name); [...] Technically this might make some LSMs unhappy, if they want to set some non-default context even if parent is all default, but this is already impossible now and in this case I think we have no better choice than sacrificing a bit of flexibility for memory efficiency, which is apparently critical here. Tejun, Casey, would the above modification be fine with you? -- Ondrej Mosnacek Associate Software Engineer, Security Technologies Red Hat, Inc.