From patchwork Tue Jan 25 22:46:25 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Berger X-Patchwork-Id: 12724329 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 1E1DCC433FE for ; Tue, 25 Jan 2022 22:47:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234342AbiAYWrr (ORCPT ); Tue, 25 Jan 2022 17:47:47 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:21076 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S234208AbiAYWr2 (ORCPT ); Tue, 25 Jan 2022 17:47:28 -0500 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 20PLGbdR031386; Tue, 25 Jan 2022 22:47:00 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=d4vmynSK4tTwTieXLkMdi3BgOmjeWvPlbFcB3Ng1q6s=; b=gmrbUFG0l3UbFfGUup1Gw4c7bYdr66IzLUnFWO4fG0GNRHgeDQ7BnoVODCDDHQz8mQHV JtDNsT26KA6t1q/8b6NQku9GqQvuKGVKPRRYKGG16J4xZqzeocEwGctbeFpmip8RuHvV bf9RKCY1h4A27Uh/Ubg3eQ48tA3fiNQXxySC9WYEIZ7cR1/G1FF5q7/CcCzPaS4UVKAP rT4ZeQMj7IKR8p20gWFL0Oe/qb9bojQ987tOAzObN1SeyXyujD30+S4wJi89PLuw7dG9 3tOAsTOGSGWZMpbawdBt95EbRFua8d/SPXeT8ZHt89NJPV9Khv0Bze9Uskkiy+S7KXIr +A== Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 3dtryvsftg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 25 Jan 2022 22:47:00 +0000 Received: from m0098413.ppops.net (m0098413.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 20PM5Iw5011912; Tue, 25 Jan 2022 22:46:59 GMT Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0b-001b2d01.pphosted.com with ESMTP id 3dtryvsftd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 25 Jan 2022 22:46:59 +0000 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 20PMhDTW008672; Tue, 25 Jan 2022 22:46:59 GMT Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by ppma04dal.us.ibm.com with ESMTP id 3dr9japd5q-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 25 Jan 2022 22:46:58 +0000 Received: from b01ledav006.gho.pok.ibm.com (b01ledav006.gho.pok.ibm.com [9.57.199.111]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 20PMku0t40108504 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 25 Jan 2022 22:46:56 GMT Received: from b01ledav006.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C2D8FAC05B; Tue, 25 Jan 2022 22:46:56 +0000 (GMT) Received: from b01ledav006.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A6F2BAC05F; Tue, 25 Jan 2022 22:46:56 +0000 (GMT) Received: from sbct-3.pok.ibm.com (unknown [9.47.158.153]) by b01ledav006.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 25 Jan 2022 22:46:56 +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, Christian Brauner Subject: [PATCH v9 03/23] securityfs: rework dentry creation Date: Tue, 25 Jan 2022 17:46:25 -0500 Message-Id: <20220125224645.79319-4-stefanb@linux.vnet.ibm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220125224645.79319-1-stefanb@linux.vnet.ibm.com> References: <20220125224645.79319-1-stefanb@linux.vnet.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: F_JPw6yKVB5Qq8YzKRvqBwtkMU88DZyq X-Proofpoint-ORIG-GUID: 4gYolXMWKJJ4xaBu_KeLF2QxjgKdfIyd X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-01-25_05,2022-01-25_02,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 clxscore=1011 lowpriorityscore=0 bulkscore=0 impostorscore=0 adultscore=0 malwarescore=0 priorityscore=1501 mlxlogscore=999 suspectscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000 definitions=main-2201250135 Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 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 --- 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);