Skip to content

Commit f75aeb2

Browse files
ameryhungAlexei Starovoitov
authored andcommitted
bpf: Dissociate struct_ops program with map if map_update fails
Currently, when bpf_struct_ops_map_update_elem() fails, the programs' st_ops_assoc will remain set. They may become dangling pointers if the map is freed later, but they will never be dereferenced since the struct_ops attachment did not succeed. However, if one of the programs is subsequently attached as part of another struct_ops map, its st_ops_assoc will be poisoned even though its old st_ops_assoc was stale from a failed attachment. Fix the spurious poisoned st_ops_assoc by dissociating struct_ops programs with a map if the attachment fails. Move bpf_prog_assoc_struct_ops() to after *plink++ to make sure bpf_prog_disassoc_struct_ops() will not miss a program when iterating st_map->links. Note that, dissociating a program from a map requires some attention as it must not reset a poisoned st_ops_assoc or a st_ops_assoc pointing to another map. The former is already guarded in bpf_prog_disassoc_struct_ops(). The latter also will not happen since st_ops_assoc of programs in st_map->links are set by bpf_prog_assoc_struct_ops(), which can only be poisoned or pointing to the current map. Signed-off-by: Amery Hung <ameryhung@gmail.com> Link: https://lore.kernel.org/r/20260417174900.2895486-1-ameryhung@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent 2845989 commit f75aeb2

1 file changed

Lines changed: 4 additions & 3 deletions

File tree

kernel/bpf/bpf_struct_ops.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,9 +811,6 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
811811
goto reset_unlock;
812812
}
813813

814-
/* Poison pointer on error instead of return for backward compatibility */
815-
bpf_prog_assoc_struct_ops(prog, &st_map->map);
816-
817814
link = kzalloc_obj(*link, GFP_USER);
818815
if (!link) {
819816
bpf_prog_put(prog);
@@ -824,6 +821,9 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
824821
&bpf_struct_ops_link_lops, prog, prog->expected_attach_type);
825822
*plink++ = &link->link;
826823

824+
/* Poison pointer on error instead of return for backward compatibility */
825+
bpf_prog_assoc_struct_ops(prog, &st_map->map);
826+
827827
ksym = kzalloc_obj(*ksym, GFP_USER);
828828
if (!ksym) {
829829
err = -ENOMEM;
@@ -906,6 +906,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
906906
reset_unlock:
907907
bpf_struct_ops_map_free_ksyms(st_map);
908908
bpf_struct_ops_map_free_image(st_map);
909+
bpf_struct_ops_map_dissoc_progs(st_map);
909910
bpf_struct_ops_map_put_progs(st_map);
910911
memset(uvalue, 0, map->value_size);
911912
memset(kvalue, 0, map->value_size);

0 commit comments

Comments
 (0)