Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…/git/bpf/bpf

Daniel Borkmann says:

====================
pull-request: bpf 2023-03-23

We've added 8 non-merge commits during the last 13 day(s) which contain
a total of 21 files changed, 238 insertions(+), 161 deletions(-).

The main changes are:

1) Fix verification issues in some BPF programs due to their stack usage
   patterns, from Eduard Zingerman.

2) Fix to add missing overflow checks in xdp_umem_reg and return an error
   in such case, from Kal Conley.

3) Fix and undo poisoning of strlcpy in libbpf given it broke builds for
   libcs which provided the former like uClibc-ng, from Jesus Sanchez-Palencia.

4) Fix insufficient bpf_jit_limit default to avoid users running into hard
   to debug seccomp BPF errors, from Daniel Borkmann.

5) Fix driver return code when they don't support a bpf_xdp_metadata kfunc
   to make it unambiguous from other errors, from Jesper Dangaard Brouer.

6) Two BPF selftest fixes to address compilation errors from recent changes
   in kernel structures, from Alexei Starovoitov.

* tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf:
  xdp: bpf_xdp_metadata use EOPNOTSUPP for no driver support
  bpf: Adjust insufficient default bpf_jit_limit
  xsk: Add missing overflow check in xdp_umem_reg
  selftests/bpf: Fix progs/test_deny_namespace.c issues.
  selftests/bpf: Fix progs/find_vma_fail1.c build error.
  libbpf: Revert poisoning of strlcpy
  selftests/bpf: Tests for uninitialized stack reads
  bpf: Allow reads from uninit stack
====================

Link: https://lore.kernel.org/r/20230323225221.6082-1-daniel@iogearbox.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Mar 23, 2023
2 parents 2e63a2d + 915efd8 commit 1b4ae19
Show file tree
Hide file tree
Showing 21 changed files with 238 additions and 161 deletions.
7 changes: 5 additions & 2 deletions Documentation/networking/xdp-rx-metadata.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ metadata is supported, this set will grow:
An XDP program can use these kfuncs to read the metadata into stack
variables for its own consumption. Or, to pass the metadata on to other
consumers, an XDP program can store it into the metadata area carried
ahead of the packet.
ahead of the packet. Not all packets will necessary have the requested
metadata available in which case the driver returns ``-ENODATA``.

Not all kfuncs have to be implemented by the device driver; when not
implemented, the default ones that return ``-EOPNOTSUPP`` will be used.
implemented, the default ones that return ``-EOPNOTSUPP`` will be used
to indicate the device driver have not implemented this kfunc.


Within an XDP frame, the metadata layout (accessed via ``xdp_buff``) is
as follows::
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/mellanox/mlx4/en_rx.c
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ int mlx4_en_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
struct mlx4_en_xdp_buff *_ctx = (void *)ctx;

if (unlikely(_ctx->ring->hwtstamp_rx_filter != HWTSTAMP_FILTER_ALL))
return -EOPNOTSUPP;
return -ENODATA;

*timestamp = mlx4_en_get_hwtstamp(_ctx->mdev,
mlx4_en_get_cqe_ts(_ctx->cqe));
Expand All @@ -686,7 +686,7 @@ int mlx4_en_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash)
struct mlx4_en_xdp_buff *_ctx = (void *)ctx;

if (unlikely(!(_ctx->dev->features & NETIF_F_RXHASH)))
return -EOPNOTSUPP;
return -ENODATA;

*hash = be32_to_cpu(_ctx->cqe->immed_rss_invalid);
return 0;
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ static int mlx5e_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
const struct mlx5e_xdp_buff *_ctx = (void *)ctx;

if (unlikely(!mlx5e_rx_hw_stamp(_ctx->rq->tstamp)))
return -EOPNOTSUPP;
return -ENODATA;

*timestamp = mlx5e_cqe_ts_to_ns(_ctx->rq->ptp_cyc2time,
_ctx->rq->clock, get_cqe_ts(_ctx->cqe));
Expand All @@ -174,7 +174,7 @@ static int mlx5e_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash)
const struct mlx5e_xdp_buff *_ctx = (void *)ctx;

if (unlikely(!(_ctx->xdp.rxq->dev->features & NETIF_F_RXHASH)))
return -EOPNOTSUPP;
return -ENODATA;

*hash = be32_to_cpu(_ctx->cqe->rss_hash_result);
return 0;
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/veth.c
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,7 @@ static int veth_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
struct veth_xdp_buff *_ctx = (void *)ctx;

if (!_ctx->skb)
return -EOPNOTSUPP;
return -ENODATA;

*timestamp = skb_hwtstamps(_ctx->skb)->hwtstamp;
return 0;
Expand All @@ -1653,7 +1653,7 @@ static int veth_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash)
struct veth_xdp_buff *_ctx = (void *)ctx;

if (!_ctx->skb)
return -EOPNOTSUPP;
return -ENODATA;

*hash = skb_get_hash(_ctx->skb);
return 0;
Expand Down
2 changes: 1 addition & 1 deletion kernel/bpf/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ static int __init bpf_jit_charge_init(void)
{
/* Only used as heuristic here to derive limit. */
bpf_jit_limit_max = bpf_jit_alloc_exec_limit();
bpf_jit_limit = min_t(u64, round_up(bpf_jit_limit_max >> 2,
bpf_jit_limit = min_t(u64, round_up(bpf_jit_limit_max >> 1,
PAGE_SIZE), LONG_MAX);
return 0;
}
Expand Down
11 changes: 10 additions & 1 deletion kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -3826,6 +3826,8 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
continue;
if (type == STACK_MISC)
continue;
if (type == STACK_INVALID && env->allow_uninit_stack)
continue;
verbose(env, "invalid read from stack off %d+%d size %d\n",
off, i, size);
return -EACCES;
Expand Down Expand Up @@ -3863,6 +3865,8 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
continue;
if (type == STACK_ZERO)
continue;
if (type == STACK_INVALID && env->allow_uninit_stack)
continue;
verbose(env, "invalid read from stack off %d+%d size %d\n",
off, i, size);
return -EACCES;
Expand Down Expand Up @@ -5754,7 +5758,8 @@ static int check_stack_range_initialized(
stype = &state->stack[spi].slot_type[slot % BPF_REG_SIZE];
if (*stype == STACK_MISC)
goto mark;
if (*stype == STACK_ZERO) {
if ((*stype == STACK_ZERO) ||
(*stype == STACK_INVALID && env->allow_uninit_stack)) {
if (clobber) {
/* helper can write anything into the stack */
*stype = STACK_MISC;
Expand Down Expand Up @@ -13936,6 +13941,10 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
if (old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_INVALID)
continue;

if (env->allow_uninit_stack &&
old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_MISC)
continue;

/* explored stack has more populated slots than current stack
* and these slots were used
*/
Expand Down
10 changes: 8 additions & 2 deletions net/core/xdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,10 @@ __diag_ignore_all("-Wmissing-prototypes",
* @ctx: XDP context pointer.
* @timestamp: Return value pointer.
*
* Returns 0 on success or ``-errno`` on error.
* Return:
* * Returns 0 on success or ``-errno`` on error.
* * ``-EOPNOTSUPP`` : means device driver does not implement kfunc
* * ``-ENODATA`` : means no RX-timestamp available for this frame
*/
__bpf_kfunc int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
{
Expand All @@ -732,7 +735,10 @@ __bpf_kfunc int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *tim
* @ctx: XDP context pointer.
* @hash: Return value pointer.
*
* Returns 0 on success or ``-errno`` on error.
* Return:
* * Returns 0 on success or ``-errno`` on error.
* * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
* * ``-ENODATA`` : means no RX-hash available for this frame
*/
__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash)
{
Expand Down
13 changes: 7 additions & 6 deletions net/xdp/xdp_umem.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,11 @@ static int xdp_umem_account_pages(struct xdp_umem *umem)

static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
{
u32 npgs_rem, chunk_size = mr->chunk_size, headroom = mr->headroom;
bool unaligned_chunks = mr->flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG;
u64 npgs, addr = mr->addr, size = mr->len;
unsigned int chunks, chunks_rem;
u32 chunk_size = mr->chunk_size, headroom = mr->headroom;
u64 addr = mr->addr, size = mr->len;
u32 chunks_rem, npgs_rem;
u64 chunks, npgs;
int err;

if (chunk_size < XDP_UMEM_MIN_CHUNK_SIZE || chunk_size > PAGE_SIZE) {
Expand Down Expand Up @@ -188,8 +189,8 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
if (npgs > U32_MAX)
return -EINVAL;

chunks = (unsigned int)div_u64_rem(size, chunk_size, &chunks_rem);
if (chunks == 0)
chunks = div_u64_rem(size, chunk_size, &chunks_rem);
if (!chunks || chunks > U32_MAX)
return -EINVAL;

if (!unaligned_chunks && chunks_rem)
Expand All @@ -202,7 +203,7 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
umem->headroom = headroom;
umem->chunk_size = chunk_size;
umem->chunks = chunks;
umem->npgs = (u32)npgs;
umem->npgs = npgs;
umem->pgs = NULL;
umem->user = NULL;
umem->flags = mr->flags;
Expand Down
4 changes: 2 additions & 2 deletions tools/lib/bpf/libbpf_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
/* make sure libbpf doesn't use kernel-only integer typedefs */
#pragma GCC poison u8 u16 u32 u64 s8 s16 s32 s64

/* prevent accidental re-addition of reallocarray()/strlcpy() */
#pragma GCC poison reallocarray strlcpy
/* prevent accidental re-addition of reallocarray() */
#pragma GCC poison reallocarray

#include "libbpf.h"
#include "btf.h"
Expand Down
9 changes: 9 additions & 0 deletions tools/testing/selftests/bpf/prog_tests/uninit_stack.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// SPDX-License-Identifier: GPL-2.0

#include <test_progs.h>
#include "uninit_stack.skel.h"

void test_uninit_stack(void)
{
RUN_TESTS(uninit_stack);
}
1 change: 1 addition & 0 deletions tools/testing/selftests/bpf/progs/find_vma_fail1.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* Copyright (c) 2021 Facebook */
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#define vm_flags vm_start

char _license[] SEC("license") = "GPL";

Expand Down
10 changes: 4 additions & 6 deletions tools/testing/selftests/bpf/progs/test_deny_namespace.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,19 @@
#include <errno.h>
#include <linux/capability.h>

struct kernel_cap_struct {
__u64 val;
} __attribute__((preserve_access_index));
typedef struct { unsigned long long val; } kernel_cap_t;

struct cred {
struct kernel_cap_struct cap_effective;
kernel_cap_t cap_effective;
} __attribute__((preserve_access_index));

char _license[] SEC("license") = "GPL";

SEC("lsm.s/userns_create")
int BPF_PROG(test_userns_create, const struct cred *cred, int ret)
{
struct kernel_cap_struct caps = cred->cap_effective;
__u64 cap_mask = BIT_LL(CAP_SYS_ADMIN);
kernel_cap_t caps = cred->cap_effective;
__u64 cap_mask = 1ULL << CAP_SYS_ADMIN;

if (ret)
return 0;
Expand Down
8 changes: 4 additions & 4 deletions tools/testing/selftests/bpf/progs/test_global_func10.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
#include "bpf_misc.h"

struct Small {
int x;
long x;
};

struct Big {
int x;
int y;
long x;
long y;
};

__noinline int foo(const struct Big *big)
Expand All @@ -22,7 +22,7 @@ __noinline int foo(const struct Big *big)
}

SEC("cgroup_skb/ingress")
__failure __msg("invalid indirect read from stack")
__failure __msg("invalid indirect access to stack")
int global_func10(struct __sk_buff *skb)
{
const struct Small small = {.x = skb->len };
Expand Down
87 changes: 87 additions & 0 deletions tools/testing/selftests/bpf/progs/uninit_stack.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// SPDX-License-Identifier: GPL-2.0

#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include "bpf_misc.h"

/* Read an uninitialized value from stack at a fixed offset */
SEC("socket")
__naked int read_uninit_stack_fixed_off(void *ctx)
{
asm volatile (" \
r0 = 0; \
/* force stack depth to be 128 */ \
*(u64*)(r10 - 128) = r1; \
r1 = *(u8 *)(r10 - 8 ); \
r0 += r1; \
r1 = *(u8 *)(r10 - 11); \
r1 = *(u8 *)(r10 - 13); \
r1 = *(u8 *)(r10 - 15); \
r1 = *(u16*)(r10 - 16); \
r1 = *(u32*)(r10 - 32); \
r1 = *(u64*)(r10 - 64); \
/* read from a spill of a wrong size, it is a separate \
* branch in check_stack_read_fixed_off() \
*/ \
*(u32*)(r10 - 72) = r1; \
r1 = *(u64*)(r10 - 72); \
r0 = 0; \
exit; \
"
::: __clobber_all);
}

/* Read an uninitialized value from stack at a variable offset */
SEC("socket")
__naked int read_uninit_stack_var_off(void *ctx)
{
asm volatile (" \
call %[bpf_get_prandom_u32]; \
/* force stack depth to be 64 */ \
*(u64*)(r10 - 64) = r0; \
r0 = -r0; \
/* give r0 a range [-31, -1] */ \
if r0 s<= -32 goto exit_%=; \
if r0 s>= 0 goto exit_%=; \
/* access stack using r0 */ \
r1 = r10; \
r1 += r0; \
r2 = *(u8*)(r1 + 0); \
exit_%=: r0 = 0; \
exit; \
"
:
: __imm(bpf_get_prandom_u32)
: __clobber_all);
}

static __noinline void dummy(void) {}

/* Pass a pointer to uninitialized stack memory to a helper.
* Passed memory block should be marked as STACK_MISC after helper call.
*/
SEC("socket")
__log_level(7) __msg("fp-104=mmmmmmmm")
__naked int helper_uninit_to_misc(void *ctx)
{
asm volatile (" \
/* force stack depth to be 128 */ \
*(u64*)(r10 - 128) = r1; \
r1 = r10; \
r1 += -128; \
r2 = 32; \
call %[bpf_trace_printk]; \
/* Call to dummy() forces print_verifier_state(..., true), \
* thus showing the stack state, matched by __msg(). \
*/ \
call %[dummy]; \
r0 = 0; \
exit; \
"
:
: __imm(bpf_trace_printk),
__imm(dummy)
: __clobber_all);
}

char _license[] SEC("license") = "GPL";
13 changes: 8 additions & 5 deletions tools/testing/selftests/bpf/verifier/calls.c
Original file line number Diff line number Diff line change
Expand Up @@ -2221,19 +2221,22 @@
* that fp-8 stack slot was unused in the fall-through
* branch and will accept the program incorrectly
*/
BPF_JMP_IMM(BPF_JGT, BPF_REG_1, 2, 2),
BPF_EMIT_CALL(BPF_FUNC_get_prandom_u32),
BPF_JMP_IMM(BPF_JGT, BPF_REG_0, 2, 2),
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
BPF_JMP_IMM(BPF_JA, 0, 0, 0),
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
BPF_LD_MAP_FD(BPF_REG_1, 0),
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
.fixup_map_hash_48b = { 6 },
.errstr = "invalid indirect read from stack R2 off -8+0 size 8",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_XDP,
.fixup_map_hash_48b = { 7 },
.errstr_unpriv = "invalid indirect read from stack R2 off -8+0 size 8",
.result_unpriv = REJECT,
/* in privileged mode reads from uninitialized stack locations are permitted */
.result = ACCEPT,
},
{
"calls: ctx read at start of subprog",
Expand Down
Loading

0 comments on commit 1b4ae19

Please sign in to comment.