Re: [PATCH bpf-next 0/1] use preserve_static_offset in bpf uapi headers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/7/23 4:05 PM, Eduard Zingerman wrote:
For certain program context types, the verifier applies the
verifier.c:convert_ctx_access() transformation.
It modifies ST/STX/LDX instructions that access program context.
convert_ctx_access() updates the offset field of these instructions
changing "virtual" offset by offset corresponding to data
representation in the running kernel.

For this transformation to be applicable access to the context field
shouldn't use pointer arithmetics. For example, consider the read of
__sk_buff->pkt_type field.
If translated as a single ST instruction:

     r0 = *(u32 *)(r1 + 4);

The verifier would accept such code and patch the offset in the
instruction, however, if translated as a pair of instructions:

     r1 += 4;
     r0 = *(u32 *)(r1 + 0);

The verifier would reject such code.

Occasionally clang shuffling code during compilation might break
verifier expectations and cause verification errors, e.g. as in [0].
Technically, this happens because each field read/write represented in
LLVM IR as two operations: address lookup + memory access,
and the compiler is free to move and substitute those independently.
For example, LLVM can rewrite C code below:

     __u32 v;
     if (...)
       v = sk_buff->pkt_type;
     else
       v = sk_buff->mark;

As if it was written as so:

     __u32 v, *p;
     if (...)
       p = &sk_buff->pkt_type;  // r0 = 4; (offset of pkt_type)
     else
       p = &sk_buff->mark;      // r0 = 8; (offset of mark)
     v = *p;                    // r1 += r0;
                                // r0 = *(u32 *)(r1 + 0)

Which is a valid rewrite from the point of view of C semantics but won't
pass verification, because convert_ctx_access() can no longer replace
offset in 'r0 = *(u32 *)(r1 + 0)' with a constant.

Recently, attribute preserve_static_offset was added to
clang [1] to tackle this problem. From its documentation:

   Clang supports the ``__attribute__((preserve_static_offset))``
   attribute for the BPF target. This attribute may be attached to a
   struct or union declaration. Reading or writing fields of types having
   such annotation is guaranteed to generate LDX/ST/STX instruction with
   an offset corresponding to the field.

The convert_ctx_access() transformation is applied when the context
parameter has one of the following types:
- __sk_buff
- bpf_cgroup_dev_ctx
- bpf_perf_event_data
- bpf_sk_lookup
- bpf_sock
- bpf_sock_addr
- bpf_sock_ops
- bpf_sockopt
- bpf_sysctl
- sk_msg_md
- sk_reuseport_md
- xdp_md

All context types are defined in include/linux/bpf_types.h.
The context type bpf_nf_ctx is missing.


 From my understanding, BPF programs typically access definitions of
these types in two ways:
- via uapi headers linux/bpf.h and linux/bpf_perf_event.h;

and bpf_nf_ctx is defined in include/net/netfilter/nf_bpf_link.h
and rely on vmlinux.h to provide the ctx struct definition.

- via vmlinux.h.

This RFC seeks to mark with preserve_static_offset the definitions of
the relevant context types within uapi headers.

The attribute is abstracted by '__bpf_ctx' macro.
As bpf.h and bpf_perf_event.h do not share any common include files,
this RFC opts to copy the same definition of '__bpf_ctx' in both
headers to avoid adding a new uapi header.
(Another tempting location for '__bpf_ctx' is compiler_types.h /
  compiler-clang.h, but these headers are not exported as uapi).

Previously I think we might use similar mechanism like vmlinux.h
with push/pop preserve_static_offset attributes. But looks like
there are many other structures in uapi bpf.h do not need
preserve_static_offset. So I think your approach sounds okay.


How to add the same definitions in vmlinux.h is an open question,
and most likely requires bpftool modification:
- Hard code generation of __bpf_ctx based on type names?
- Mark context types with some special
   __attribute__((btf_decl_tag("preserve_static_offset")))
   and convert it to __attribute__((preserve_static_offset))?

The number of context types is limited, I would just go through
the first approach with hard coding the list of ctx types and
mark them with preserve_static_offset attribute in vmlinux.h.


Please suggest if any of the options above sound reasonable.

[0] https://lore.kernel.org/bpf/CAA-VZPmxh8o8EBcJ=m-DH4ytcxDFmo0JKsm1p1gf40kS0CE3NQ@xxxxxxxxxxxxxx/T/#m4b9ce2ce73b34f34172328f975235fc6f19841b6
[1] 030b8cb1561d ("[BPF] Attribute preserve_static_offset for structs")
     git@xxxxxxxxxx:llvm/llvm-project.git

Eduard Zingerman (1):
   bpf: Mark virtual BPF context structures as preserve_static_offset

  include/uapi/linux/bpf.h                  | 28 ++++++++++++++---------
  include/uapi/linux/bpf_perf_event.h       |  8 ++++++-
  tools/include/uapi/linux/bpf.h            | 28 ++++++++++++++---------
  tools/include/uapi/linux/bpf_perf_event.h |  8 ++++++-
  4 files changed, 48 insertions(+), 24 deletions(-)





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux