Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 004778bf authored by He Fengqing's avatar He Fengqing Committed by Greg Kroah-Hartman
Browse files

bpf: Fix potential memleak and UAF in the verifier. 



[ Upstream commit 75f0fc7b48ad45a2e5736bcf8de26c8872fe8695 ]

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


Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent fa4802c5

kernel/bpf/verifier.c

+16 −11
Original line number Diff line number Diff line
@@ -8401,10 +8401,11 @@ static void convert_pseudo_ld_imm64(struct bpf_verifier_env *env) 
 * 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

 */

static int adjust_insn_aux_data(struct bpf_verifier_env *env,

static void adjust_insn_aux_data(struct bpf_verifier_env *env,

				 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;

	bool old_seen = old_data[off].seen;

	u32 prog_len;
@@ -8417,12 +8418,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);



	if (cnt == 1)

		return 0;

		return;

	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 + off + cnt - 1, old_data + off,

	       sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
@@ -8433,7 +8431,6 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env, 
	}

	env->insn_aux_data = new_data;

	vfree(old_data);

	return 0;

}



static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len)
@@ -8454,6 +8451,14 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of 
					    const struct bpf_insn *patch, u32 len)

{

	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);

	if (IS_ERR(new_prog)) {
@@ -8461,10 +8466,10 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of 
			verbose(env,

				"insn %d cannot be patched due to 16-bit range\n",

				env->insn_aux_data[off].orig_idx);

		vfree(new_data);

		return NULL;

	}

	if (adjust_insn_aux_data(env, new_prog, off, len))

		return NULL;

	adjust_insn_aux_data(env, new_data, new_prog, off, len);

	adjust_subprog_starts(env, off, len);

	return new_prog;

}