Re: [PATCH 4.19 84/86] efi/x86: Handle by-ref arguments covering multiple pages in mixed mode

[Pavel Machek]

Hi!

> Currently, the mixed mode runtime service wrappers require that all by-ref
> arguments that live in the vmalloc space have a size that is a power of 2,
> and are aligned to that same value. While this is a sensible way to
> construct an object that is guaranteed not to cross a page boundary, it is
> overly strict when it comes to checking whether a given object violates
> this requirement, as we can simply take the physical address of the first
> and the last byte, and verify that they point into the same physical
> page.

Dunno. If start passing buffers that _sometime_ cross page boundaries,
we'll get hard to debug failures. Maybe original code is better
buecause it catches problems earlier?

Furthermore, all existing code should pass aligned, 2^n size buffers,
so we should not need this in stable?

> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -321,16 +321,13 @@ virt_to_phys_or_null_size(void *va, unsi
>  	if (virt_addr_valid(va))
>  		return virt_to_phys(va);
>  
> -	/*
> -	 * A fully aligned variable on the stack is guaranteed not to
> -	 * cross a page bounary. Try to catch strings on the stack by
> -	 * checking that 'size' is a power of two.
> -	 */
> -	bad_size = size > PAGE_SIZE || !is_power_of_2(size);
> +	pa = slow_virt_to_phys(va);
>  
> -	WARN_ON(!IS_ALIGNED((unsigned long)va, size) || bad_size);
> +	/* check if the object crosses a page boundary */
> +	if (WARN_ON((pa ^ (pa + size - 1)) & PAGE_MASK))
> +		return 0;

We don't really need to do this computation on pa, it would work on va
as well, right? It does not matter much, but old code worked that way.

Plus, strictly speaking, pa + size can overflow for huge sizes, and
test will return false negatives.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: PGP signature