Subject: Re: [PATCH v17 08/13] namei: LOOKUP_BENEATH: O_BENEATH-like scoped resolution

From: Al Viro <[email-masked]>
Date: Mon, 25 Nov 2019 00:26:04 +0000

On Sun, Nov 17, 2019 at 12:17:08PM +1100, Aleksa Sarai wrote:

> +	if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> +		/*
> +		 * Do a final check to ensure that the path didn't escape. Note
> +		 * that this should already be guaranteed by all of the other
> +		 * LOOKUP_IS_SCOPED checks (and delaying this check this late
> +		 * does open the door to some possible timing-based attacks).
> +		 */
> +		if (WARN_ON(!path_is_under(&nd->path, &nd->root)))
> +			return -EXDEV;

I don't like that.  What it gives is an ability to race that with
rename(), with user-triggered WARN_ON.  You *can't* promise that result of
lookup is in a subtree, simply because it can get moved just as you've
declared it to be in the clear.

	Anyone who relies upon that is delusional; it really can't be done.
What warranties LOOKUP_IS_SCOPED is really supposed to provide?  That we do not
attempt to walk out of the subtree rooted at the start point?  Fine, but this
is not what this test does.  What are you trying to achieve there?  If it's
"what we'd got was at one point in our subtree", the test is more or less
right, but WARN_ON isn't.