From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [62.89.141.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 299ED79F5; Sat, 15 Jun 2024 06:55:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=62.89.141.173 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718434545; cv=none; b=i1n+0g3rlfUIBOOD0QiSL1R32ZyhdZ0kslkWW5m4E5a22A/Xu30SbXHmAVnJgrtecQ70T/Z22v6hDKnmVK0NmbPUuOwczH9GZcwDMs8dTGplANPwu0Tkie6+G5c3mZakEUnR74qB5vytO3pkjOSlryircPQ6vN6fHHbaFwh6fMY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718434545; c=relaxed/simple; bh=v0MCPzGmDHWaCmXNTaeRXiP4uAHdb48+oNgoKNrIOUk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=euGq6GfpOgU5SwFx2KNd0qx7nGu0HybiXPMKhomezAJz6+uQyaP443GhvRKCYtKhn/T1jTGnQTSA870nm3QOo3jyPQe+mebrvz+Q4NNxaRLLL7/sAmpZUGMJN09WIO1qmzcdgqgnVSUowgExYQ0+mENZrVUuQd8C+ELq5+LU1b8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=zeniv.linux.org.uk; spf=none smtp.mailfrom=ftp.linux.org.uk; dkim=pass (2048-bit key) header.d=linux.org.uk header.i=@linux.org.uk header.b=rDSyu5OC; arc=none smtp.client-ip=62.89.141.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=zeniv.linux.org.uk Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=ftp.linux.org.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linux.org.uk header.i=@linux.org.uk header.b="rDSyu5OC" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=92LCsaXkJeoRVpJsDtxyMK6znw0YY1VAEpYw5A4ddpc=; b=rDSyu5OCFt2rJ1qjMvxp0ICybI 0pTc7tvz9ZDGbi8VtwA3pG1ZrjPUtwN30w2vc0oDyAZjRLi49wyONs4KPqVtJfp1EKHfRdKfMRL8p SertZn86m2xBe2vVqWGZQCy/hfEJKHymekRsQlrrvhUm/bubHlm9IOAg7OT7CSeCl8LWpSEsEEmM+ 2YH7cErZdtF1pGJNdIewTFetlcweD5A3qp0vHlVJh8rB85vEXPfU46zZbDv25Q6ebnPtGZkYAw5un 8K63EQPNyJD8vK5jSAeHMBSSYtvSIU/RTFZKHwbTqYHpVFhVZG/Kw2tAPzN9+1tOxvZF6fB0N/cXM xLnTX97g==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1sINJw-009tiR-1d; Sat, 15 Jun 2024 06:55:28 +0000 Date: Sat, 15 Jun 2024 07:55:28 +0100 From: Al Viro To: Christoph Hellwig Cc: Congjie Zhou , brauner@kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] fs: modify the annotation of vfs_mkdir() in fs/namei.c Message-ID: <20240615065528.GP1629371@ZenIV> References: <20240615030056.GO1629371@ZenIV> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: Al Viro On Fri, Jun 14, 2024 at 11:29:38PM -0700, Christoph Hellwig wrote: > On Sat, Jun 15, 2024 at 12:38:32PM +0800, Congjie Zhou wrote: > > modify the annotation of @dir and @dentry > > Well, it is clearly obvious that you modify them from the patch. But > why? Look at the current comment: * @dir: inode of @dentry It is an inode of _some_ dentry; it's most definitely not that of the argument named 'dentry'. * @dentry: pointer to dentry of the base directory No. The first thing vfs_mkdir() does is int error = may_create(idmap, dir, dentry); if (error) return error; Look at may_create(). Starts with static inline int may_create(struct mnt_idmap *idmap, struct inode *dir, struct dentry *child) { audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE); if (child->d_inode) return -EEXIST; If the last argument (aka. 'dentry' argument of vfs_mkdir()) is currently referring to *ANY* directory, you get -EEXIST. For a good and simple reason: it is the dentry of subdirectory to be created. Now, the second argument of vfs_mkdir() ('dir') is the inode of the parent to be (or base directory, if you will). While we are at it, the rest of comments coming from the same commit suffer similar problems. vfs_create(), vfs_symlink(), et.al. vfs_unlink() is fine, vfs_rmdir() should match vfs_unlink() (inode of parent + dentry of victim).