linux-kernel - Re: [PATCH v2 3/3] selftests/mm: virtual_address

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] [day] [month] [year] [list]

Message-ID: <e97c2a5d-c815-4936-a767-ac42a3220a90@redhat.com>
Date: Mon, 13 Jan 2025 10:30:55 +0100
From: David Hildenbrand <david@...hat.com>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Shuah Khan <shuah@...nel.org>,
 Dev Jain <dev.jain@....com>, Thomas Gleixner <tglx@...utronix.de>,
 linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH v2 3/3] selftests/mm: virtual_address_range: Avoid reading
 VVAR mappings

On 13.01.25 10:09, Thomas Weißschuh wrote:
> On Fri, Jan 10, 2025 at 04:41:03PM +0100, David Hildenbrand wrote:
>> On 10.01.25 14:05, Thomas Weißschuh wrote:
>>> The virtual_address_range selftest reads from the start of each mapping
>>> listed in /proc/self/maps.
>>> However not all mappings are valid to be arbitrarily accessed.
>>> For example the vvar data used for virtual clocks on x86 [vvar_vclock]
>>> can only be accessed if 1) the kernel configuration enables virtual
>>> clocks and 2) the hypervisor provided the data for it.
>>> Only the VDSO itself has the necessary information to know this.
>>> Since commit e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping")
>>> the virtual clock data was split out into its own mapping, leading
>>> to EFAULT from read() during the validation.
>>>
>>> Skip the various vvar mappings in virtual_address_range to avoid the issue.
>>>
>>> Fixes: e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping")
>>> Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()")
>>> Reported-by: kernel test robot <oliver.sang@...el.com>
>>> Closes: https://lore.kernel.org/oe-lkp/202412271148.2656e485-lkp@intel.com
>>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
>>> ---
>>>    tools/testing/selftests/mm/virtual_address_range.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
>>> index 4fc1c21a5e218eaec4d059b75c31a21dd4e8a215..993990aba56fc986c42084ffa91973558aa07e87 100644
>>> --- a/tools/testing/selftests/mm/virtual_address_range.c
>>> +++ b/tools/testing/selftests/mm/virtual_address_range.c
>>> @@ -152,6 +152,10 @@ static int validate_complete_va_space(void)
>>>    		if (prot[0] != 'r')
>>>    			continue;
>>> +		/* Only the VDSO can know if a VVAR mapping is really readable */
>>> +		if (vma_name && !strncmp(vma_name, "[vvar", 5))
>>> +			continue;
>>
>> I'm wondering if there is a more generic way ... but likely not when staring
>> at /proc/self/maps.
>>
>> /proc/self/smaps would indicate this as
>>
>> VM_IO: "io"
>> VM_DONTDUMP: "dd"
>> VM_PFNMAP: "pf"
>>
>> Especially checking for VM_IO sounds reasonable ...
> 
> Agreed.
> 
> Can we instead rely on madvise(MADV_DOFORK) returning EINVAL iff VM_IO?

That might be one option indeed, although it feels like this is 
something that might change in the future ... not sure.

MADV_POPULATE_READ fails with -EFAULT on VM_IO, but also on VM_PFNMAP
and some other conditions, and might not be completely what we want.

We do have some initial smaps parsing in vm_util.c:__check_huge(), maybe 
that could be factored out / reused, not sure how hard that would be to 
extend to return the flags.

-- 
Cheers,

David / dhildenb