• Yonghong Song's avatar
    bpf: Fail verification for sign-extension of packet data/data_end/data_meta · 92de3608
    Yonghong Song authored
    syzbot reported a kernel crash due to
      commit 1f1e864b ("bpf: Handle sign-extenstin ctx member accesses").
    The reason is due to sign-extension of 32-bit load for
    packet data/data_end/data_meta uapi field.
    
    The original code looks like:
            r2 = *(s32 *)(r1 + 76) /* load __sk_buff->data */
            r3 = *(u32 *)(r1 + 80) /* load __sk_buff->data_end */
            r0 = r2
            r0 += 8
            if r3 > r0 goto +1
            ...
    Note that __sk_buff->data load has 32-bit sign extension.
    
    After verification and convert_ctx_accesses(), the final asm code looks like:
            r2 = *(u64 *)(r1 +208)
            r2 = (s32)r2
            r3 = *(u64 *)(r1 +80)
            r0 = r2
            r0 += 8
            if r3 > r0 goto pc+1
            ...
    Note that 'r2 = (s32)r2' may make the kernel __sk_buff->data address invalid
    which may cause runtime failure.
    
    Currently, in C code, typically we have
            void *data = (void *)(long)skb->data;
            void *data_end = (void *)(long)skb->data_end;
            ...
    and it will generate
            r2 = *(u64 *)(r1 +208)
            r3 = *(u64 *)(r1 +80)
            r0 = r2
            r0 += 8
            if r3 > r0 goto pc+1
    
    If we allow sign-extension,
            void *data = (void *)(long)(int)skb->data;
            void *data_end = (void *)(long)skb->data_end;
            ...
    the generated code looks like
            r2 = *(u64 *)(r1 +208)
            r2 <<= 32
            r2 s>>= 32
            r3 = *(u64 *)(r1 +80)
            r0 = r2
            r0 += 8
            if r3 > r0 goto pc+1
    and this will cause verification failure since "r2 <<= 32" is not allowed
    as "r2" is a packet pointer.
    
    To fix this issue for case
      r2 = *(s32 *)(r1 + 76) /* load __sk_buff->data */
    this patch added additional checking in is_valid_access() callback
    function for packet data/data_end/data_meta access. If those accesses
    are with sign-extenstion, the verification will fail.
    
      [1] https://lore.kernel.org/bpf/000000000000c90eee061d236d37@google.com/
    
    Reported-by: syzbot+ad9ec60c8eaf69e6f99c@syzkaller.appspotmail.com
    Fixes: 1f1e864b ("bpf: Handle sign-extenstin ctx member accesses")
    Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
    Signed-off-by: default avatarYonghong Song <yonghong.song@linux.dev>
    Link: https://lore.kernel.org/r/20240723153439.2429035-1-yonghong.song@linux.devSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
    92de3608
verifier.c 656 KB