Commit 75f0fc7b authored by He Fengqing's avatar He Fengqing Committed by Alexei Starovoitov

bpf: Fix potential memleak and UAF in the verifier.

In bpf_patch_insn_data(), we first use the bpf_patch_insn_single() to
insert new instructions, then use adjust_insn_aux_data() to adjust
insn_aux_data. If the old env->prog have no enough room for new inserted
instructions, we use bpf_prog_realloc to construct new_prog and free the
old env->prog.

There have two errors here. First, if adjust_insn_aux_data() return
ENOMEM, we should free the new_prog. Second, if adjust_insn_aux_data()
return ENOMEM, bpf_patch_insn_data() will return NULL, and env->prog has
been freed in bpf_prog_realloc, but we will use it in bpf_check().

So in this patch, we make the adjust_insn_aux_data() never fails. In
bpf_patch_insn_data(), we first pre-malloc memory for the new
insn_aux_data, then call bpf_patch_insn_single() to insert new
instructions, at last call adjust_insn_aux_data() to adjust
insn_aux_data.

Fixes: 8041902d ("bpf: adjust insn_aux_data when patching insns")
Signed-off-by: default avatarHe Fengqing <hefengqing@huawei.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Acked-by: default avatarSong Liu <songliubraving@fb.com>
Link: https://lore.kernel.org/bpf/20210714101815.164322-1-hefengqing@huawei.com
parent f170acda
...@@ -11425,10 +11425,11 @@ static void convert_pseudo_ld_imm64(struct bpf_verifier_env *env) ...@@ -11425,10 +11425,11 @@ static void convert_pseudo_ld_imm64(struct bpf_verifier_env *env)
* insni[off, off + cnt). Adjust corresponding insn_aux_data by copying * insni[off, off + cnt). Adjust corresponding insn_aux_data by copying
* [0, off) and [off, end) to new locations, so the patched range stays zero * [0, off) and [off, end) to new locations, so the patched range stays zero
*/ */
static int adjust_insn_aux_data(struct bpf_verifier_env *env, static void adjust_insn_aux_data(struct bpf_verifier_env *env,
struct bpf_prog *new_prog, u32 off, u32 cnt) struct bpf_insn_aux_data *new_data,
struct bpf_prog *new_prog, u32 off, u32 cnt)
{ {
struct bpf_insn_aux_data *new_data, *old_data = env->insn_aux_data; struct bpf_insn_aux_data *old_data = env->insn_aux_data;
struct bpf_insn *insn = new_prog->insnsi; struct bpf_insn *insn = new_prog->insnsi;
u32 old_seen = old_data[off].seen; u32 old_seen = old_data[off].seen;
u32 prog_len; u32 prog_len;
...@@ -11441,12 +11442,9 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env, ...@@ -11441,12 +11442,9 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env,
old_data[off].zext_dst = insn_has_def32(env, insn + off + cnt - 1); old_data[off].zext_dst = insn_has_def32(env, insn + off + cnt - 1);
if (cnt == 1) if (cnt == 1)
return 0; return;
prog_len = new_prog->len; prog_len = new_prog->len;
new_data = vzalloc(array_size(prog_len,
sizeof(struct bpf_insn_aux_data)));
if (!new_data)
return -ENOMEM;
memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off); memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
memcpy(new_data + off + cnt - 1, old_data + off, memcpy(new_data + off + cnt - 1, old_data + off,
sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1)); sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
...@@ -11457,7 +11455,6 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env, ...@@ -11457,7 +11455,6 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env,
} }
env->insn_aux_data = new_data; env->insn_aux_data = new_data;
vfree(old_data); vfree(old_data);
return 0;
} }
static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len) static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len)
...@@ -11492,6 +11489,14 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of ...@@ -11492,6 +11489,14 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
const struct bpf_insn *patch, u32 len) const struct bpf_insn *patch, u32 len)
{ {
struct bpf_prog *new_prog; struct bpf_prog *new_prog;
struct bpf_insn_aux_data *new_data = NULL;
if (len > 1) {
new_data = vzalloc(array_size(env->prog->len + len - 1,
sizeof(struct bpf_insn_aux_data)));
if (!new_data)
return NULL;
}
new_prog = bpf_patch_insn_single(env->prog, off, patch, len); new_prog = bpf_patch_insn_single(env->prog, off, patch, len);
if (IS_ERR(new_prog)) { if (IS_ERR(new_prog)) {
...@@ -11499,10 +11504,10 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of ...@@ -11499,10 +11504,10 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
verbose(env, verbose(env,
"insn %d cannot be patched due to 16-bit range\n", "insn %d cannot be patched due to 16-bit range\n",
env->insn_aux_data[off].orig_idx); env->insn_aux_data[off].orig_idx);
vfree(new_data);
return NULL; return NULL;
} }
if (adjust_insn_aux_data(env, new_prog, off, len)) adjust_insn_aux_data(env, new_data, new_prog, off, len);
return NULL;
adjust_subprog_starts(env, off, len); adjust_subprog_starts(env, off, len);
adjust_poke_descs(new_prog, off, len); adjust_poke_descs(new_prog, off, len);
return new_prog; return new_prog;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment