Received-SPF: pass (google.com: domain of linux-kernel+bounces-16192-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 4 Jan 2024 01:59:10 +0000
From: Al Viro <viro@zeniv.linux.org.uk>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linux Trace Kernel <linux-trace-kernel@vger.kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christian Brauner <brauner@kernel.org>,
	linux-fsdevel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default
 ownership
Message-ID: <20240104015910.GP1674809@ZenIV>
References: <20240103203246.115732ec@gandalf.local.home>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240103203246.115732ec@gandalf.local.home>
Sender: Al Viro <viro@ftp.linux.org.uk>

On Wed, Jan 03, 2024 at 08:32:46PM -0500, Steven Rostedt wrote:

> +static struct inode *instance_inode(struct dentry *parent, struct inode *inode)
> +{
> +	struct tracefs_inode *ti;
> +	struct inode *root_inode;
> +
> +	root_inode = d_inode(inode->i_sb->s_root);
> +
> +	/* If parent is NULL then use root inode */
> +	if (!parent)
> +		return root_inode;
> +
> +	/* Find the inode that is flagged as an instance or the root inode */
> +	do {
> +		inode = d_inode(parent);
> +		if (inode == root_inode)
> +			return root_inode;
> +
> +		ti = get_tracefs(inode);
> +
> +		if (ti->flags & TRACEFS_INSTANCE_INODE)
> +			return inode;
> +	} while ((parent = parent->d_parent));

*blink*

This is equivalent to
		...
		parent = parent->d_parent;
	} while (true);

->d_parent is *never* NULL.  And what the hell is that loop supposed to do,
anyway?  Find the nearest ancestor tagged with TRACEFS_INSTANCE_INODE?

If root is not marked that way, I would suggest
	if (!parent)
		parent = inode->i_sb->s_root;
	while (!IS_ROOT(parent)) {
		struct tracefs_inode *ti = get_tracefs(parent->d_inode);
		if (ti->flags & TRACEFS_INSTANCE_INODE)
			break;
		parent = parent->d_parent;
	}
	return parent->d_inode;