On 3/15/24 11:11 AM, Kui-Feng Lee wrote:
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -742,8 +742,11 @@ 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 && !err; i++)
+ err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
+
+ if (err)
nit: Can it be more specific? I mean to check err < 0, so we can reason
that this function never returns a positive value other than 0.
I think "if (err)" is fine. It is pretty common in other places of the kernel.
Checking "(err < 0)" may actually mean the return value could be positive. At
least it is how bpf_struct_ops.c is using "(err < 0)".
[ An unrelated side note is another (err < 0) check in bpf_struct_ops.c could
have been changed after the recent changes in bpf_struct_ops_prepare_trampoline
which no longer return +val ].
