bpf: Check return from set_memory_rox()

arch_protect_bpf_trampoline() and alloc_new_pack() call
set_memory_rox() which can fail, leading to unprotected memory.

Take into account return from set_memory_rox() function and add
__must_check flag to arch_protect_bpf_trampoline().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/fe1c163c83767fde5cab31d209a4a6be3ddb3a73.1710574353.git.christophe.leroy@csgroup.eu
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 132c8ffba10962bb97ca8a7ef6685722b9a2a8d6..bc16eb6946578bebe2933aa5fe1ee7f9cb77faf0 100644 (file)
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2176,8 +2176,9 @@ void arch_free_bpf_trampoline(void *image, unsigned int size)
        bpf_prog_pack_free(image, size);
 }
 
-void arch_protect_bpf_trampoline(void *image, unsigned int size)
+int arch_protect_bpf_trampoline(void *image, unsigned int size)
 {
+       return 0;
 }
 
 int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *ro_image,

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 7a56d2d845120b1eca0ce7c88ed1d0918c49e2cb..4900b1ee019fd1c408c8bc5cf393d3a1342f0fcd 100644 (file)
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -3004,8 +3004,9 @@ void arch_free_bpf_trampoline(void *image, unsigned int size)
        bpf_prog_pack_free(image, size);
 }
 
-void arch_protect_bpf_trampoline(void *image, unsigned int size)
+int arch_protect_bpf_trampoline(void *image, unsigned int size)
 {
+       return 0;
 }
 
 int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d89bdefb42e2ff3fe77fc20634b00f2dcd478cb6..17843e66a1d3e1f9c8a2c1c80b7e749a51269160 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1116,7 +1116,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
                                void *func_addr);
 void *arch_alloc_bpf_trampoline(unsigned int size);
 void arch_free_bpf_trampoline(void *image, unsigned int size);
-void arch_protect_bpf_trampoline(void *image, unsigned int size);
+int __must_check arch_protect_bpf_trampoline(void *image, unsigned int size);
 int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
                             struct bpf_tramp_links *tlinks, void *func_addr);
 

diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 3fcd35314ce593022f2bc0bd849e5fd4aab338a4..86c7884abaf87ab1cc0d4b83b564bba584772205 100644 (file)
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -740,8 +740,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
                if (err)
                        goto reset_unlock;
        }
-       for (i = 0; i < st_map->image_pages_cnt; i++)
-               arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
+       for (i = 0; i < st_map->image_pages_cnt; i++) {
+               err = arch_protect_bpf_trampoline(st_map->image_pages[i],
+                                                 PAGE_SIZE);
+               if (err)
+                       goto reset_unlock;
+       }
 
        if (st_map->map.map_flags & BPF_F_LINK) {
                err = 0;

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 63f100def31bcee9dac2df5c25aa5288ea23f29f..5aacb1d3c4cc79c693e8606c3bba841fcfa507d6 100644 (file)
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -908,23 +908,30 @@ static LIST_HEAD(pack_list);
 static struct bpf_prog_pack *alloc_new_pack(bpf_jit_fill_hole_t bpf_fill_ill_insns)
 {
        struct bpf_prog_pack *pack;
+       int err;
 
        pack = kzalloc(struct_size(pack, bitmap, BITS_TO_LONGS(BPF_PROG_CHUNK_COUNT)),
                       GFP_KERNEL);
        if (!pack)
                return NULL;
        pack->ptr = bpf_jit_alloc_exec(BPF_PROG_PACK_SIZE);
-       if (!pack->ptr) {
-               kfree(pack);
-               return NULL;
-       }
+       if (!pack->ptr)
+               goto out;
        bpf_fill_ill_insns(pack->ptr, BPF_PROG_PACK_SIZE);
        bitmap_zero(pack->bitmap, BPF_PROG_PACK_SIZE / BPF_PROG_CHUNK_SIZE);
-       list_add_tail(&pack->list, &pack_list);
 
        set_vm_flush_reset_perms(pack->ptr);
-       set_memory_rox((unsigned long)pack->ptr, BPF_PROG_PACK_SIZE / PAGE_SIZE);
+       err = set_memory_rox((unsigned long)pack->ptr,
+                            BPF_PROG_PACK_SIZE / PAGE_SIZE);
+       if (err)
+               goto out;
+       list_add_tail(&pack->list, &pack_list);
        return pack;
+
+out:
+       bpf_jit_free_exec(pack->ptr);
+       kfree(pack);
+       return NULL;
 }
 
 void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
@@ -939,9 +946,16 @@ void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
                size = round_up(size, PAGE_SIZE);
                ptr = bpf_jit_alloc_exec(size);
                if (ptr) {
+                       int err;
+
                        bpf_fill_ill_insns(ptr, size);
                        set_vm_flush_reset_perms(ptr);
-                       set_memory_rox((unsigned long)ptr, size / PAGE_SIZE);
+                       err = set_memory_rox((unsigned long)ptr,
+                                            size / PAGE_SIZE);
+                       if (err) {
+                               bpf_jit_free_exec(ptr);
+                               ptr = NULL;
+                       }
                }
                goto out;
        }

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 04fd1abd36612cf74f8745f4f2ad7237273947d9..cc50607f8d8cadc76ee41ce88e4575400d89f455 100644 (file)
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -456,7 +456,9 @@ again:
        if (err < 0)
                goto out_free;
 
-       arch_protect_bpf_trampoline(im->image, im->size);
+       err = arch_protect_bpf_trampoline(im->image, im->size);
+       if (err)
+               goto out_free;
 
        WARN_ON(tr->cur_image && total == 0);
        if (tr->cur_image)
@@ -1072,10 +1074,10 @@ void __weak arch_free_bpf_trampoline(void *image, unsigned int size)
        bpf_jit_free_exec(image);
 }
 
-void __weak arch_protect_bpf_trampoline(void *image, unsigned int size)
+int __weak arch_protect_bpf_trampoline(void *image, unsigned int size)
 {
        WARN_ON_ONCE(size > PAGE_SIZE);
-       set_memory_rox((long)image, 1);
+       return set_memory_rox((long)image, 1);
 }
 
 int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,

diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
index de33dc1b0daadc0258950bee1adcc1cd99512143..25b75844891a7f0d1b17be5a1ed0202285f6f777 100644 (file)
--- a/net/bpf/bpf_dummy_struct_ops.c
+++ b/net/bpf/bpf_dummy_struct_ops.c
@@ -133,7 +133,9 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
        if (err < 0)
                goto out;
 
-       arch_protect_bpf_trampoline(image, PAGE_SIZE);
+       err = arch_protect_bpf_trampoline(image, PAGE_SIZE);
+       if (err)
+               goto out;
        prog_ret = dummy_ops_call_op(image, args);
 
        err = dummy_ops_copy_args(args);