Alignment check in tnum_is_aligned()

Alignment check in tnum_is_aligned()

[Bogdan Harjoc]

Hello,

bpf programs that call lock_xadd() on pointers obtained from bpf
helpers fail to load because tnum_is_aligned() returns false during
bpf validation. Should tnum_is_aligned() evaluate a.value & a.mask
instead of a.value | a.mask ?

An example bpf tracing snippet that fails validation is:

    struct task_struct *t = (struct task_struct *)bpf_get_current_task();
    lock_xadd(&t->usage.refs.counter, 1);

I noticed using a kprobe (listed below) that tnum_is_aligned()
receives value=0, mask=0xffffffffffffffff and returns 0 for the
lock_xadd() call above.

=====
b = BPF(text=R"""
#include <uapi/linux/ptrace.h>
int in_tnum_is_aligned(struct pt_regs *regs) {
    bpf_trace_printk("in value=%llx mask=%llx\n", regs->di, regs->si);
    return 0;
}
int out_tnum_is_aligned(struct pt_regs *regs) {
    bpf_trace_printk("out aligned=%llx\n", regs->ax);
    return 0;
}
""")

b.attach_kprobe(event="tnum_is_aligned", fn_name="in_fn")
b.attach_kretprobe(event="tnum_is_aligned", fn_name="out_fn")
b.trace_print(fmt="{1} {5}")
=====

Cheers!

Re: Alignment check in tnum_is_aligned()

[Daniel Borkmann]

On 2/27/20 6:07 PM, Bogdan Harjoc wrote:
> Hello,
> 
> bpf programs that call lock_xadd() on pointers obtained from bpf
> helpers fail to load because tnum_is_aligned() returns false during
> bpf validation. Should tnum_is_aligned() evaluate a.value & a.mask
> instead of a.value | a.mask ?
> 
> An example bpf tracing snippet that fails validation is:
> 
>      struct task_struct *t = (struct task_struct *)bpf_get_current_task();
>      lock_xadd(&t->usage.refs.counter, 1);

Note that bumping the refcount on bpf_get_current_task() pointer is not
possible from BPF.

> I noticed using a kprobe (listed below) that tnum_is_aligned()
> receives value=0, mask=0xffffffffffffffff and returns 0 for the
> lock_xadd() call above.

The implementation is ...

bool tnum_is_aligned(struct tnum a, u64 size)
{
         if (!size)
                 return true;
         return !((a.value | a.mask) & (size - 1));
}

... an a.value=0, a.mask=0xffffffffffffffff means that the verifier knows
nothing about the returned value from bpf_get_current_task(), so it could
be any pointer value in the whole u64 range. The masking for alignment is
based on the size arg above, not a.mask. Also bpf_get_current_task() does not
return a pointer type from verifier pov, so there's no check_generic_ptr_alignment()
test performed.

Cheers,
Daniel