Re: [PATCH v2 bpf-next] selftests/bpf: Use _REGION1_SIZE in test_snprintf_btf on s390

[Heiko Carstens <hca@linux.ibm.com>]

On Fri, Feb 26, 2021 at 07:20:14PM +0100, Ilya Leoshkevich wrote:
> test_snprintf_btf fails on s390, because NULL points to a readable
> struct lowcore there. Fix by using _REGION1_SIZE instead.
> 
> Error message example:
> 
>     printing 0000000000000000 should generate error, got (361)
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> 
> v1: https://lore.kernel.org/bpf/20210226135923.114211-1-iii@linux.ibm.com/
> v1 -> v2: Yonghong suggested to add the pointer value to the error
>           message.
>           I've noticed that I've been passing BADPTR as flags, therefore
>           the fix worked only by accident. Put it into p.ptr where it
>           belongs.
> 
>  .../testing/selftests/bpf/progs/netif_receive_skb.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> index 6b670039ea67..4d158de73c2d 100644
> --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> @@ -16,6 +16,13 @@ bool skip = false;
>  #define STRSIZE			2048
>  #define EXPECTED_STRSIZE	256
> 
> +#if defined(bpf_target_s390)
> +/* NULL points to a readable struct lowcore on s390, so take _REGION1_SIZE */
> +#define BADPTR			((void *)(1ULL << 53))
> +#else
> +#define BADPTR			0
> +#endif
> +
>  #ifndef ARRAY_SIZE
>  #define ARRAY_SIZE(x)	(sizeof(x) / sizeof((x)[0]))
>  #endif
> @@ -113,11 +120,11 @@ int BPF_PROG(trace_netif_receive_skb, struct sk_buff *skb)
>  	}
> 
>  	/* Check invalid ptr value */
> -	p.ptr = 0;
> +	p.ptr = BADPTR;
>  	__ret = bpf_snprintf_btf(str, STRSIZE, &p, sizeof(p), 0);
>  	if (__ret >= 0) {
> -		bpf_printk("printing NULL should generate error, got (%d)",
> -			   __ret);
> +		bpf_printk("printing %p should generate error, got (%d)",
> +			   BADPTR, __ret);
>  		ret = -ERANGE;

This will work for now on s390, since _right now_ we don't map
anything that high, but there is no guarantee that it will stay
this way.
I'd rather skip this test for s390.