| Message ID | 160097751992.13115.10446086919232254389.stgit@john-Precision-5820-Tower |
|---|---|
| State | New |
| Headers | show |
| Series | [bpf-next] bpf: Add comment to document BTF type PTR_TO_BTF_ID_OR_NULL | expand |
On Thu, 24 Sep 2020 at 20:58, John Fastabend <john.fastabend@gmail.com> wrote: > > The meaning of PTR_TO_BTF_ID_OR_NULL differs slightly from other types > denoted with the *_OR_NULL type. For example the types PTR_TO_SOCKET > and PTR_TO_SOCKET_OR_NULL can be used for branch analysis because the > type PTR_TO_SOCKET is guaranteed to _not_ have a null value. > > In contrast PTR_TO_BTF_ID and BTF_TO_BTF_ID_OR_NULL have slightly > different meanings. A PTR_TO_BTF_TO_ID may be a pointer to NULL value, > but it is safe to read this pointer in the program context because > the program context will handle any faults. The fallout is for > PTR_TO_BTF_ID the verifier can assume reads are safe, but can not > use the type in branch analysis. Additionally, authors need to be > extra careful when passing PTR_TO_BTF_ID into helpers. In general > helpers consuming type PTR_TO_BTF_ID will need to assume it may > be null. > > Seeing the above is not obvious to readers without the back knowledge > lets add a comment in the type definition. > > Editorial comment, as networking and tracing programs get closer > and more tightly merged we may need to consider a new type that we > can ensure is non-null for branch analysis and also passing into > helpers. Yeah, I was going back and forth with Martin on this as well. I think we need better descriptions for possibly-NULL-at-runtime for the purpose of helper call invariants, and possibly-NULL-at-verification time. > > Signed-off-by: John Fastabend <john.fastabend@gmail.com> Acked-by: Lorenz Bauer <lmb@cloudflare.com> > --- > include/linux/bpf.h | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index fc5c901c7542..dd765ba1c730 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -382,8 +382,22 @@ enum bpf_reg_type { > PTR_TO_TCP_SOCK_OR_NULL, /* reg points to struct tcp_sock or NULL */ > PTR_TO_TP_BUFFER, /* reg points to a writable raw tp's buffer */ > PTR_TO_XDP_SOCK, /* reg points to struct xdp_sock */ > - PTR_TO_BTF_ID, /* reg points to kernel struct */ > - PTR_TO_BTF_ID_OR_NULL, /* reg points to kernel struct or NULL */ > + /* PTR_TO_BTF_ID points to a kernel struct that does not need > + * to be null checked by the BPF program. This does not imply the > + * pointer is _not_ null and in practice this can easily be a null > + * pointer when reading pointer chains. The assumption is program > + * context will handle null pointer dereference typically via fault > + * handling. The verifier must keep this in mind and can make no > + * assumptions about null or non-null when doing branch analysis. > + * Further, when passed into helpers the helpers can not, without > + * additional context, assume the value is non-null. > + */ > + PTR_TO_BTF_ID, > + /* PTR_TO_BTF_ID_OR_NULL points to a kernel struct that has not > + * been checked for null. Used primarily to inform the verifier > + * an explicit null check is required for this struct. > + */ > + PTR_TO_BTF_ID_OR_NULL, > PTR_TO_MEM, /* reg points to valid memory region */ > PTR_TO_MEM_OR_NULL, /* reg points to valid memory region or NULL */ > PTR_TO_RDONLY_BUF, /* reg points to a readonly buffer */ > -- Lorenz Bauer | Systems Engineer 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK www.cloudflare.com
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index fc5c901c7542..dd765ba1c730 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -382,8 +382,22 @@ enum bpf_reg_type { PTR_TO_TCP_SOCK_OR_NULL, /* reg points to struct tcp_sock or NULL */ PTR_TO_TP_BUFFER, /* reg points to a writable raw tp's buffer */ PTR_TO_XDP_SOCK, /* reg points to struct xdp_sock */ - PTR_TO_BTF_ID, /* reg points to kernel struct */ - PTR_TO_BTF_ID_OR_NULL, /* reg points to kernel struct or NULL */ + /* PTR_TO_BTF_ID points to a kernel struct that does not need + * to be null checked by the BPF program. This does not imply the + * pointer is _not_ null and in practice this can easily be a null + * pointer when reading pointer chains. The assumption is program + * context will handle null pointer dereference typically via fault + * handling. The verifier must keep this in mind and can make no + * assumptions about null or non-null when doing branch analysis. + * Further, when passed into helpers the helpers can not, without + * additional context, assume the value is non-null. + */ + PTR_TO_BTF_ID, + /* PTR_TO_BTF_ID_OR_NULL points to a kernel struct that has not + * been checked for null. Used primarily to inform the verifier + * an explicit null check is required for this struct. + */ + PTR_TO_BTF_ID_OR_NULL, PTR_TO_MEM, /* reg points to valid memory region */ PTR_TO_MEM_OR_NULL, /* reg points to valid memory region or NULL */ PTR_TO_RDONLY_BUF, /* reg points to a readonly buffer */
The meaning of PTR_TO_BTF_ID_OR_NULL differs slightly from other types denoted with the *_OR_NULL type. For example the types PTR_TO_SOCKET and PTR_TO_SOCKET_OR_NULL can be used for branch analysis because the type PTR_TO_SOCKET is guaranteed to _not_ have a null value. In contrast PTR_TO_BTF_ID and BTF_TO_BTF_ID_OR_NULL have slightly different meanings. A PTR_TO_BTF_TO_ID may be a pointer to NULL value, but it is safe to read this pointer in the program context because the program context will handle any faults. The fallout is for PTR_TO_BTF_ID the verifier can assume reads are safe, but can not use the type in branch analysis. Additionally, authors need to be extra careful when passing PTR_TO_BTF_ID into helpers. In general helpers consuming type PTR_TO_BTF_ID will need to assume it may be null. Seeing the above is not obvious to readers without the back knowledge lets add a comment in the type definition. Editorial comment, as networking and tracing programs get closer and more tightly merged we may need to consider a new type that we can ensure is non-null for branch analysis and also passing into helpers. Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- include/linux/bpf.h | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)