From patchwork Thu Jul 7 14:47:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Berger X-Patchwork-Id: 12909771 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E919C43334 for ; Thu, 7 Jul 2022 14:50:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236042AbiGGOuB (ORCPT ); Thu, 7 Jul 2022 10:50:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41968 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236111AbiGGOtO (ORCPT ); Thu, 7 Jul 2022 10:49:14 -0400 Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 678603334E; Thu, 7 Jul 2022 07:48:55 -0700 (PDT) Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 267EMeMm022481; Thu, 7 Jul 2022 14:48:07 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=whl2GHMtNnzVs294V7Y+yivlxzZK4XMiEqwPy/cTN9Q=; b=O/PcZGWQopdZ1VPeDcDD0s/mwR8zQ5YnwZk0j+fIJFSSeSYh48y6T88L2dhv3HaXBhob Q1po/qT7srAZiCDGL5k+q7G88bBFl8vg6kjjqMsZGNPcxmgQA/hglbgmOgzLpZ/omUNi zDsyn/2uXBF7Fxj8cABrI9clJC/5RuRtx9JjPtsexCy74QsT0deArpvVkx7W5z+VwABh RVGHvR93Jiq3MvtSl6L4JpA22k+YIsitoQ4IYTRkEIX2ucEQYH9zYxyeAkQT5IJzmSIQ nzhbxmOxtzzUksuFAEzYetvNP2mSxCX4tFt2FzZ+mFlbOHzaQe55vVcB+MuGOt/QfhzC 2Q== Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3h612q1a2d-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 07 Jul 2022 14:48:07 +0000 Received: from m0098416.ppops.net (m0098416.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 267EMubB025034; Thu, 7 Jul 2022 14:48:06 GMT Received: from ppma01dal.us.ibm.com (83.d6.3fa9.ip4.static.sl-reverse.com [169.63.214.131]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3h612q1a1d-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 07 Jul 2022 14:48:06 +0000 Received: from pps.filterd (ppma01dal.us.ibm.com [127.0.0.1]) by ppma01dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 267EZkOT021769; Thu, 7 Jul 2022 14:48:05 GMT Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by ppma01dal.us.ibm.com with ESMTP id 3h4ud7pvt5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 07 Jul 2022 14:48:05 +0000 Received: from b01ledav004.gho.pok.ibm.com (b01ledav004.gho.pok.ibm.com [9.57.199.109]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 267Em4a849938832 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 7 Jul 2022 14:48:04 GMT Received: from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 06F23112061; Thu, 7 Jul 2022 14:48:04 +0000 (GMT) Received: from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E442E11206B; Thu, 7 Jul 2022 14:48:03 +0000 (GMT) Received: from sbct-3.pok.ibm.com (unknown [9.47.158.153]) by b01ledav004.gho.pok.ibm.com (Postfix) with ESMTP; Thu, 7 Jul 2022 14:48:03 +0000 (GMT) From: Stefan Berger To: linux-integrity@vger.kernel.org Cc: zohar@linux.ibm.com, serge@hallyn.com, christian.brauner@ubuntu.com, containers@lists.linux.dev, dmitry.kasatkin@gmail.com, ebiederm@xmission.com, krzysztof.struczynski@huawei.com, roberto.sassu@huawei.com, mpeters@redhat.com, lhinds@redhat.com, lsturman@redhat.com, puiterwi@redhat.com, jejb@linux.ibm.com, jamjoom@us.ibm.com, linux-kernel@vger.kernel.org, paul@paul-moore.com, rgb@redhat.com, linux-security-module@vger.kernel.org, jmorris@namei.org, jpenumak@redhat.com, Christian Brauner , John Johansen , Matthew Garrett , Micah Morton , Kentaro Takeda , Jarkko Sakkinen , Stefan Berger Subject: [PATCH v13 01/26] securityfs: rework dentry creation Date: Thu, 7 Jul 2022 10:47:35 -0400 Message-Id: <20220707144800.828288-2-stefanb@linux.ibm.com> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220707144800.828288-1-stefanb@linux.ibm.com> References: <20220707144800.828288-1-stefanb@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: ZYlNjviV1gEGNCfvAH7On83EmwXAUkb2 X-Proofpoint-ORIG-GUID: KfY7TFXEa9EUch8275NFJsIE25pl0-ac X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-07-07_12,2022-06-28_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 phishscore=0 mlxlogscore=999 spamscore=0 lowpriorityscore=0 suspectscore=0 priorityscore=1501 mlxscore=0 adultscore=0 clxscore=1011 bulkscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000 definitions=main-2207070057 Precedence: bulk List-ID: From: Christian Brauner When securityfs creates a new file or directory via securityfs_create_dentry() it will take an additional reference on the newly created dentry after it has attached the new inode to the new dentry and added it to the hashqueues. If we contrast this with debugfs, which has the same underlying logic as securityfs, it uses a similar pairing as securityfs. Where securityfs has the securityfs_create_dentry() and securityfs_remove() pairing, debugfs has the __debugfs_create_file() and debugfs_remove() pairing. In contrast to securityfs, debugfs doesn't take an additional reference on the newly created dentry in __debugfs_create_file() which would need to be put in debugfs_remove(). The additional dget() isn't a problem per se. In the current implementation of securityfs each created dentry pins the filesystem via securityfs_create_dentry() until it is removed. Since it is virtually guaranteed that there is at least one user of securityfs that has created dentries the initial securityfs mount cannot go away until all dentries have been removed. Since most of the users of the initial securityfs mount don't go away until the system is shutdown the initial securityfs won't go away when unmounted. Instead a mount will usually surface the same superblock as before. The additional dget() doesn't matter in this scenario since it is required that all dentries have been cleaned up by the respective users before the superblock can be destroyed, i.e. superblock shutdown is tied to the lifetime of the associated dentries. However, in order to support ima namespaces we need to extend securityfs to support being mounted outside of the initial user namespace. For namespaced users the pinning logic doesn't make sense. Whereas in the initial namespace the securityfs instance and the associated data structures of its users can't go away for reason explained earlier users of non-initial securityfs instances do go away when the last users of the namespace are gone. So for those users we neither want to duplicate the pinning logic nor make the global securityfs instance display different information based on the namespace. Both options would be really messy and hacky. Instead we will simply give each namespace its own securityfs instance similar to how each ipc namespace has its own mqueue instance and all entries in there are cleaned up on umount or when the last user of the associated namespace is gone. This means that the superblock's lifetime isn't tied to the dentries. Instead the last umount, without any fds kept open, will trigger a clean shutdown. But now the additional dget() gets in the way. Instead of being able to rely on the generic superblock shutdown logic we would need to drop the additional dentry reference during superblock shutdown for all associated users. That would force the use of a generic coordination mechanism for current and future users of securityfs which is unnecessary. Simply remove the additional dget() in securityfs_dentry_create(). In securityfs_remove() we will call dget() to take an additional reference on the dentry about to be removed. After simple_unlink() or simple_rmdir() have dropped the dentry refcount we can call d_delete() which will either turn the dentry into negative dentry if our earlier dget() is the only reference to the dentry, i.e. it has no other users, or remove it from the hashqueues in case there are additional users. All of these changes should not have any effect on the userspace semantics of the initial securityfs mount. Signed-off-by: Christian Brauner Cc: John Johansen Cc: Matthew Garrett Cc: Micah Morton Cc: Kentaro Takeda Cc: James Morris Cc: Jarkko Sakkinen Signed-off-by: Stefan Berger Reviewed-by: Mimi Zohar Reviewed-by: Serge Hallyn --- v13: - Slight improvements in 1st paragraph of commit message --- security/inode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/inode.c b/security/inode.c index 6c326939750d..13e6780c4444 100644 --- a/security/inode.c +++ b/security/inode.c @@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, inode->i_fop = fops; } d_instantiate(dentry, inode); - dget(dentry); inode_unlock(dir); return dentry; @@ -302,10 +301,12 @@ void securityfs_remove(struct dentry *dentry) dir = d_inode(dentry->d_parent); inode_lock(dir); if (simple_positive(dentry)) { + dget(dentry); if (d_is_dir(dentry)) simple_rmdir(dir, dentry); else simple_unlink(dir, dentry); + d_delete(dentry); dput(dentry); } inode_unlock(dir);