On Wed, Dec 19, 2018 at 10:29:18AM -0800, Jakub Kicinski wrote:
> Instead of overwriting dead code with jmp -1 instructions
> remove it completely for root. Adjust verifier state and
> line info appropriately.
>
> Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> ---
> include/linux/filter.h | 1 +
> kernel/bpf/core.c | 12 ++++
> kernel/bpf/verifier.c | 139 ++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 149 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 537e9e7c6e6f..c99969022493 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -782,6 +782,7 @@ static inline bool bpf_dump_raw_ok(void)
>
> struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
> const struct bpf_insn *patch, u32 len);
> +int bpf_remove_insns(struct bpf_prog *prog, u32 off, u32 cnt);
>
> void bpf_clear_redirect_map(struct bpf_map *map);
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 2eb7cc9822bb..e3498d82eb74 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -461,6 +461,18 @@ struct bpf_prog *bpf_patch_insn_single(struct bpf_prog
> *prog, u32 off,
> return prog_adj;
> }
>
> +int bpf_remove_insns(struct bpf_prog *prog, u32 off, u32 cnt)
> +{
> + /* Branch offsets can't overflow when program is shrinking, no need
> + * to call bpf_adj_branches(..., true) here
> + */
> + memmove(prog->insnsi + off, prog->insnsi + off + cnt,
> + sizeof(struct bpf_insn) * (prog->len - off - cnt));
> + prog->len -= cnt;
> +
> + return WARN_ON_ONCE(bpf_adj_branches(prog, off, off + cnt, off, false));
> +}
> +
> void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp)
> {
> int i;
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7db2a48ea7f7..98a276f4c4e6 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6226,6 +6226,113 @@ static struct bpf_prog *bpf_patch_insn_data(struct
> bpf_verifier_env *env, u32 of
> return new_prog;
> }
>
> +static int adjust_subprog_starts_after_remove(struct bpf_verifier_env *env,
> + u32 off, u32 cnt)
> +{
> + int i, j;
> +
> + /* find first prog starting at or after off (first to remove) */
> + for (i = 0; i < env->subprog_cnt; i++)
> + if (env->subprog_info[i].start >= off)
> + break;
> + /* find first prog starting at or after off + cnt (first to stay) */
> + for (j = i; j < env->subprog_cnt; j++)
> + if (env->subprog_info[j].start >= off + cnt)
> + break;
> + /* if j doesn't start exactly at off + cnt, we are just removing
> + * the front of previous prog
> + */
I think this will break func_info, since it's not adjusted here.
Also iirc line_info is relying on first insn to always have line_info.
If first insn is dead, second insn might not have a line_info generated
and things won't go well.
Martin, Yonghong, please chime in.
