On 3/17/23 11:10, Martin KaFai Lau wrote:
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.
Sound good! I will move this line.
+
+ 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;
+}
