On Fri, 2021-02-26 at 19:47 -0800, Yonghong Song wrote:

On 2/26/21 11:09 AM, Ilya Leoshkevich wrote:

quoted

test_snprintf_btf fails on s390, because NULL points to a readable
struct lowcore there. Fix by using the last page 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/ (local)
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.

v2: 
https://lore.kernel.org/bpf/20210226182014.115347-1-iii@linux.ibm.com/ (local)
v2 -> v3: Heiko mentioned that using _REGION1_SIZE is not future-
proof.
           We had a private discussion and came to the conclusion
that
           the the last page is good enough.

Heiko, could you ack the patch if it is okay? Thanks!

quoted

  .../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..c3669967067e 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 the
last page */
+#define BADPTR                 ((void *)0xFFFFFFFFFFFFF000ULL)
+#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);

 From https://www.kernel.org/doc/Documentation/printk-formats.txt:

Pointers printed without a specifier extension (i.e unadorned %p) are
hashed to give a unique identifier without leaking kernel addresses
to user
space. On 64 bit machines the first 32 bits are zeroed. If you
_really_
want the address see %px below.

I think it is okay to use %px here.

I don't think bpf_trace_printk supports it, but I'll use %llx instead.

[...]