On 3/1/22 4:43 PM, Song Liu wrote:
On do_jit failure path, the header is freed by bpf_jit_binary_pack_free.
While bpf_jit_binary_pack_free doesn't require proper ro_header->size,
bpf_prog_pack_free still uses it. Set header->size in bpf_int_jit_compile
before calling bpf_jit_binary_pack_free.
Fixes: 1022a5498f6f ("bpf, x86_64: Use bpf_jit_binary_pack_alloc")
Fixes: 33c9805860e5 ("bpf: Introduce bpf_jit_binary_pack_[alloc|finalize|free]")
Reported-by: Kui-Feng Lee <kuifeng@xxxxxx>
Signed-off-by: Song Liu <song@xxxxxxxxxx>
LGTM with a nit below related to comments.
Acked-by: Yonghong Song <yhs@xxxxxx>
---
arch/x86/net/bpf_jit_comp.c | 6 +++++-
kernel/bpf/core.c | 7 ++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index c7db0fe4de2f..b923d81ff6f9 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2330,8 +2330,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
if (proglen <= 0) {
out_image:
image = NULL;
- if (header)
+ if (header) {
+ /* set header->size for bpf_arch_text_copy */
This comment is confusing. Setting header->size is not for
bpf_arch_text_copy. Probably you mean 'by bpf_arch_text_copy?
I think this comment is not necessary.
+ bpf_arch_text_copy(&header->size, &rw_header->size,
+ sizeof(rw_header->size));
bpf_jit_binary_pack_free(header, rw_header);
+ }
prog = orig_prog;
goto out_addrs;
}
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ebb0193d07f0..da587e4619e0 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1112,13 +1112,14 @@ int bpf_jit_binary_pack_finalize(struct bpf_prog *prog,
* 1) when the program is freed after;
* 2) when the JIT engine fails (before bpf_jit_binary_pack_finalize).
* For case 2), we need to free both the RO memory and the RW buffer.
- * Also, ro_header->size in 2) is not properly set yet, so rw_header->size
- * is used for uncharge.
+ *
+ * If bpf_jit_binary_pack_free is called before _finalize (jit error),
Do you mean bpf_jit_binary_pack_free() is called before calling
bpf_jit_binary_pack_finalize()? This seems not the case.
+ * it is necessary to set ro_header->size properly before freeing it.
*/
void bpf_jit_binary_pack_free(struct bpf_binary_header *ro_header,
struct bpf_binary_header *rw_header)
{
- u32 size = rw_header ? rw_header->size : ro_header->size;
+ u32 size = ro_header->size;
bpf_prog_pack_free(ro_header);
kvfree(rw_header);
