Re: [PATCH v3] vsprintf: Make sure argument to %pX specifier is valid

[Julien Grall <julien.grall@linaro.org>]

Hi Boris,

On 18/02/2015 15:39, Boris Ostrovsky wrote:
> If invalid pointer (i.e. something smaller than HYPERVISOR_VIRT_START)
> is passed for %*ph/%pv/%ps/%pS format specifiers then print value of the
> pointer in parentheses.
>
> For example:
>
>   struct vcpu *v0 = NULL;
>   struct vcpu *v1 = (void *)0xffUL;
>   unsigned val = 0xab;
>   unsigned *ptr = &val;
>   unsigned *badptr = (void *)0xab;
>   printk("v0 = %pv, v1 = %pv, curr = %pv\n", v0, v1, current);
>   printk("badptr = %*ph, ptr = %*ph\n", 1, badptr, 1, ptr);
>
> will produce
>   v0 = (0), v1 = (ff), curr = d0v3
>   badptr = (ab), ptr = ab
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
>   xen/common/vsprintf.c |   23 ++++++++++++++++++++++-
>   1 files changed, 22 insertions(+), 1 deletions(-)
>
> v3:
>   * Print value of the bad pointer in parentheses.
>     (I understand Andrew's dislike of additional switch but I
>     think this is the cleanest way)
>
> v2:
>   * Print "(NULL)" instead of specifier-specific string
>   * Consider all addresses under HYPERVISOR_VIRT_START as invalid. (I think
>     this is true for both x86 and ARM but I don't have ARM platform to test).

This assumption is valid on ARM too. Although, we may have some mappings 
after HYPERVISOR_VIRT_START why are not valid.

On ARM, HYPERVISOR_VIRT_END marks the end of mapping which is not always 
mapped (such as the domheap). Would it make sense to test it? Although 
it seems that x86 doesn't have the same meaning for this macro.

Regards,

-- 
Julien Grall