Re: untagged_addr_remote() in do_madvise()

[Dave Hansen <dave.hansen@intel.com>]

On 1/14/25 11:43, Liam R. Howlett wrote:
> Can anyone tell me why the code today is correct?  That is, how can we
> trust the validation of start/end is still okay after we change the
> start/end by untagging the start?

Well, let's walk through the start/end validation.  First:

        if (!PAGE_ALIGNED(start))
                return -EINVAL;

That should stay valid as long as the tag/untag doesn't affect the lower
bits. All the tagging is upper bits that are far away from the
<PAGE_SHIFT bits. I can hardly imagine an implementation that would tag
in the lower bits.

        end = start + len;
        if (end < start)
                return -EINVAL;

This one is a test for negative 'len' and for start+len overflows. It's
certainly possible that a tagged 'start' would overflow when an untagged
'start' would not. But something that overflows that positive/negative
boundary would also cross a tag boundary so it would probably be a bug
anyway.

The last check is:

        if (end == start)
                return 0;

But since 'end' is derived from 'start':

	end = start + len;

I can't think of a way that changing 'start'  (via untagging) will end
up changing the result 'end==start' comparison.

So, is the code "correct"?  The overflow detection can certainly be
triggered with tagged addresses, but it's arguably doing a service in
that case since the input in nonsensical crossing tags.

I'd say it's correct, but far from *obviously* correct. It could
definitely use some clarity.