linux-kernel - Re: untagged_addr_remote() in do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1053174c-0057-4086-b8c2-360eb2504104@intel.com>
Date: Tue, 14 Jan 2025 12:15:30 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>, dave.hansen@...ux.intel.com,
 kirill.shutemov@...ux.intel.com, Shakeel Butt <shakeel.butt@...ux.dev>,
 SeongJae Park <sj@...nel.org>, David Hildenbrand <david@...hat.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, Andrew Morton <akpm@...ux-foundation.org>,
 Jens Axboe <axboe@...nel.dk>, Pavel Begunkov <asml.silence@...il.com>,
 linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 Ryan Roberts <ryan.roberts@....com>
Subject: Re: untagged_addr_remote() in do_madvise()

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.