* [PATCH bpf] xdp: use trusted arguments in XDP hints kfuncs
@ 2023-07-11 10:59 Larysa Zaremba
2023-07-11 14:21 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 4+ messages in thread
From: Larysa Zaremba @ 2023-07-11 10:59 UTC (permalink / raw)
To: bpf, Stanislav Fomichev
Cc: Larysa Zaremba, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Eric Dumazet, Paolo Abeni, Martin KaFai Lau,
netdev, linux-kernel
Currently, verifier does not reject XDP programs that pass NULL pointer to
hints functions. At the same time, this case is not handled in any driver
implementation (including veth). For example, changing
bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
to
bpf_xdp_metadata_rx_timestamp(ctx, NULL);
in xdp_metadata test successfully crashes the system.
Add KF_TRUSTED_ARGS flag to hints kfunc definitions, so driver code
does not have to worry about getting invalid pointers.
Fixes: 3d76a4d3d4e5 ("bpf: XDP metadata RX kfuncs")
Reported-by: Stanislav Fomichev <sdf@google.com>
Closes: https://lore.kernel.org/bpf/ZKWo0BbpLfkZHbyE@google.com/
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
net/core/xdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 41e5ca8643ec..8362130bf085 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -741,7 +741,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash,
__diag_pop();
BTF_SET8_START(xdp_metadata_kfunc_ids)
-#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, 0)
+#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
XDP_METADATA_KFUNC_xxx
#undef XDP_METADATA_KFUNC
BTF_SET8_END(xdp_metadata_kfunc_ids)
--
2.41.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] xdp: use trusted arguments in XDP hints kfuncs
2023-07-11 10:59 [PATCH bpf] xdp: use trusted arguments in XDP hints kfuncs Larysa Zaremba
@ 2023-07-11 14:21 ` Jesper Dangaard Brouer
2023-07-11 17:00 ` Stanislav Fomichev
0 siblings, 1 reply; 4+ messages in thread
From: Jesper Dangaard Brouer @ 2023-07-11 14:21 UTC (permalink / raw)
To: Larysa Zaremba, bpf, Stanislav Fomichev
Cc: brouer, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
Eric Dumazet, Paolo Abeni, Martin KaFai Lau, netdev,
linux-kernel
On 11/07/2023 12.59, Larysa Zaremba wrote:
> Currently, verifier does not reject XDP programs that pass NULL pointer to
> hints functions. At the same time, this case is not handled in any driver
> implementation (including veth). For example, changing
>
> bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
>
> to
>
> bpf_xdp_metadata_rx_timestamp(ctx, NULL);
>
> in xdp_metadata test successfully crashes the system.
>
> Add KF_TRUSTED_ARGS flag to hints kfunc definitions, so driver code
> does not have to worry about getting invalid pointers.
>
Looks good to me, assuming this means verifier will reject BPF-prog's
supplying NULL.
Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
> Fixes: 3d76a4d3d4e5 ("bpf: XDP metadata RX kfuncs")
> Reported-by: Stanislav Fomichev <sdf@google.com>
> Closes: https://lore.kernel.org/bpf/ZKWo0BbpLfkZHbyE@google.com/
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
> net/core/xdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 41e5ca8643ec..8362130bf085 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -741,7 +741,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash,
> __diag_pop();
>
> BTF_SET8_START(xdp_metadata_kfunc_ids)
> -#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, 0)
> +#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
> XDP_METADATA_KFUNC_xxx
> #undef XDP_METADATA_KFUNC
> BTF_SET8_END(xdp_metadata_kfunc_ids)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] xdp: use trusted arguments in XDP hints kfuncs
2023-07-11 14:21 ` Jesper Dangaard Brouer
@ 2023-07-11 17:00 ` Stanislav Fomichev
2023-07-12 3:06 ` Alexei Starovoitov
0 siblings, 1 reply; 4+ messages in thread
From: Stanislav Fomichev @ 2023-07-11 17:00 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Larysa Zaremba, bpf, brouer, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Eric Dumazet, Paolo Abeni, Martin KaFai Lau,
netdev, linux-kernel
On Tue, Jul 11, 2023 at 7:21 AM Jesper Dangaard Brouer
<jbrouer@redhat.com> wrote:
>
>
> On 11/07/2023 12.59, Larysa Zaremba wrote:
> > Currently, verifier does not reject XDP programs that pass NULL pointer to
> > hints functions. At the same time, this case is not handled in any driver
> > implementation (including veth). For example, changing
> >
> > bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
> >
> > to
> >
> > bpf_xdp_metadata_rx_timestamp(ctx, NULL);
> >
> > in xdp_metadata test successfully crashes the system.
> >
> > Add KF_TRUSTED_ARGS flag to hints kfunc definitions, so driver code
> > does not have to worry about getting invalid pointers.
> >
>
> Looks good to me, assuming this means verifier will reject BPF-prog's
> supplying NULL.
>
> Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
>
> > Fixes: 3d76a4d3d4e5 ("bpf: XDP metadata RX kfuncs")
> > Reported-by: Stanislav Fomichev <sdf@google.com>
> > Closes: https://lore.kernel.org/bpf/ZKWo0BbpLfkZHbyE@google.com/
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
Acked-by: Stanislav Fomichev <sdf@google.com>
Thank you for the fix!
> > ---
> > net/core/xdp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 41e5ca8643ec..8362130bf085 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -741,7 +741,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash,
> > __diag_pop();
> >
> > BTF_SET8_START(xdp_metadata_kfunc_ids)
> > -#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, 0)
> > +#define XDP_METADATA_KFUNC(_, name) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
> > XDP_METADATA_KFUNC_xxx
> > #undef XDP_METADATA_KFUNC
> > BTF_SET8_END(xdp_metadata_kfunc_ids)
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] xdp: use trusted arguments in XDP hints kfuncs
2023-07-11 17:00 ` Stanislav Fomichev
@ 2023-07-12 3:06 ` Alexei Starovoitov
0 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2023-07-12 3:06 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Jesper Dangaard Brouer, Larysa Zaremba, bpf,
Jesper Dangaard Brouer, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Eric Dumazet, Paolo Abeni, Martin KaFai Lau,
Network Development, LKML
On Tue, Jul 11, 2023 at 10:00 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> On Tue, Jul 11, 2023 at 7:21 AM Jesper Dangaard Brouer
> <jbrouer@redhat.com> wrote:
> >
> >
> > On 11/07/2023 12.59, Larysa Zaremba wrote:
> > > Currently, verifier does not reject XDP programs that pass NULL pointer to
> > > hints functions. At the same time, this case is not handled in any driver
> > > implementation (including veth). For example, changing
> > >
> > > bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
> > >
> > > to
> > >
> > > bpf_xdp_metadata_rx_timestamp(ctx, NULL);
> > >
> > > in xdp_metadata test successfully crashes the system.
> > >
> > > Add KF_TRUSTED_ARGS flag to hints kfunc definitions, so driver code
> > > does not have to worry about getting invalid pointers.
> > >
> >
> > Looks good to me, assuming this means verifier will reject BPF-prog's
> > supplying NULL.
> >
> > Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
> >
> > > Fixes: 3d76a4d3d4e5 ("bpf: XDP metadata RX kfuncs")
> > > Reported-by: Stanislav Fomichev <sdf@google.com>
> > > Closes: https://lore.kernel.org/bpf/ZKWo0BbpLfkZHbyE@google.com/
> > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
>
> Acked-by: Stanislav Fomichev <sdf@google.com>
>
> Thank you for the fix!
Applied. Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-12 3:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-11 10:59 [PATCH bpf] xdp: use trusted arguments in XDP hints kfuncs Larysa Zaremba
2023-07-11 14:21 ` Jesper Dangaard Brouer
2023-07-11 17:00 ` Stanislav Fomichev
2023-07-12 3:06 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®