On 3/15/23 7:36 PM, Kui-Feng Lee wrote:
+int bpf_struct_ops_link_create(union bpf_attr *attr) +{ + struct bpf_struct_ops_link *link = NULL; + struct bpf_link_primer link_primer; + struct bpf_struct_ops_map *st_map; + struct bpf_map *map; + int err; + + map = bpf_map_get(attr->link_create.map_fd); + if (!map) + return -EINVAL; + + st_map = (struct bpf_struct_ops_map *)map; + + if (!bpf_struct_ops_valid_to_reg(map)) { + err = -EINVAL; + goto err_out; + } + + link = kzalloc(sizeof(*link), GFP_USER); + if (!link) { + err = -ENOMEM; + goto err_out; + } + bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS, &bpf_struct_ops_map_lops, NULL); + RCU_INIT_POINTER(link->map, map);
The link->map assignment should be done with the bpf_link_settle(), meaning only assign after everything else has succeeded.
The link is not exposed to user space until bpf_link_settle(). The link->map assignment can be done after ->reg succeeded and do it just before bpf_link_settle(). Then there is no need to do the RCU_INIT_POINTER(link->map, NULL) dance in the error case.
+ + err = bpf_link_prime(&link->link, &link_primer); + if (err) + goto err_out; + + err = st_map->st_ops->reg(st_map->kvalue.data); + if (err) { + /* No RCU since no one has a chance to read this pointer yet. */ + RCU_INIT_POINTER(link->map, NULL); + bpf_link_cleanup(&link_primer); + link = NULL; + goto err_out; + } + + return bpf_link_settle(&link_primer); + +err_out: + bpf_map_put(map); + kfree(link); + return err; +}