On Fri, 2021-02-26 at 19:47 -0800, Yonghong Song wrote: > > > On 2/26/21 11:09 AM, Ilya Leoshkevich wrote: > > 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@xxxxxxxxxxxxx> > > --- > > > > v1: > > https://lore.kernel.org/bpf/20210226135923.114211-1-iii@xxxxxxxxxxxxx/ > > 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@xxxxxxxxxxxxx/ > > 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! > > > > > .../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. [...]
