Skip to contentSkip to conversation

libbpf: fix race when pinning maps in parallel

PATCH Applied Martynas Pumputis 3 participants 3 messages Jul 26, 2021 – Jul 27, 2021

Subsystems (2)
BPF libbpf
lore
Share & tools
lore

Martynas Pumputis

author
#0 lore raw

PATCH v2 1/1 libbpf: fix race when pinning maps in parallel

When loading in parallel multiple programs which use the same to-be pinned map, it is possible that two instances of the loader will call bpf_object__create_maps() at the same time. If the map doesn't exist when both instances call bpf_object__reuse_map(), then one of the instances will fail with EEXIST when calling bpf_map__pin().

Fix the race by retrying reusing a map if bpf_map__pin() returns EEXIST. The fix is similar to the one in iproute2: e4c4685fd6e4 ("bpf:
Fix race condition with map pinning").

Before retrying the pinning, we don't do any special cleaning of an internal map state. The closer code inspection revealed that it's not required:

- bpf_object__create_map(): map->inner_map is destroyed after a successful call, map->fd is closed if pinning fails.
- bpf_object__populate_internal_map(): created map elements is destroyed upon close(map->fd).
- init_map_slots(): slots are freed after their initialization.

Cc Joe Stringer <[email protected]>
Signed-off-by Martynas Pumputis <[email protected]>
---
 tools/lib/bpf/libbpf.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
tools/lib/bpf/libbpf.c +14 -1
index 6f5e2757bb3c..9a39453bb45b 100644
@@ -4616,10 +4616,13 @@ bpf_object__create_maps(struct bpf_object *obj)
 	char *cp, errmsg[STRERR_BUFSIZE];
 	unsigned int i, j;
 	int err;
+	bool retried;
 
 	for (i = 0; i < obj->nr_maps; i++) {
 		map = &obj->maps[i];
 
+		retried = false;
+retry:
 		if (map->pin_path) {
 			err = bpf_object__reuse_map(map);
 			if (err) {

@@ -4627,6 +4630,12 @@ bpf_object__create_maps(struct bpf_object *obj)
 					map->name);
 				goto err_out;
 			}
+			if (retried && map->fd < 0) {
+				pr_warn("map '%s': cannot find pinned map\n",
+					map->name);
+				err = -ENOENT;
+				goto err_out;
+			}
 		}
 
 		if (map->fd >= 0) {

@@ -4660,9 +4669,13 @@ bpf_object__create_maps(struct bpf_object *obj)
 		if (map->pin_path && !map->pinned) {
 			err = bpf_map__pin(map, NULL);
 			if (err) {
+				zclose(map->fd);
+				if (!retried && err == -EEXIST) {
+					retried = true;
+					goto retry;
+				}
 				pr_warn("map '%s': failed to auto-pin at '%s': %d\n",
 					map->name, map->pin_path, err);
-				zclose(map->fd);
 				goto err_out;
 			}
 		}
Signature · 1 line
2.32.0
Applied
#1 lore raw
Quoted 21 lines · Martynas Pumputis

When loading in parallel multiple programs which use the same to-be pinned map, it is possible that two instances of the loader will call bpf_object__create_maps() at the same time. If the map doesn't exist when both instances call bpf_object__reuse_map(), then one of the instances will fail with EEXIST when calling bpf_map__pin().

Fix the race by retrying reusing a map if bpf_map__pin() returns EEXIST. The fix is similar to the one in iproute2: e4c4685fd6e4 ("bpf:
Fix race condition with map pinning").

Before retrying the pinning, we don't do any special cleaning of an internal map state. The closer code inspection revealed that it's not required:

- bpf_object__create_map(): map->inner_map is destroyed after a successful call, map->fd is closed if pinning fails.
- bpf_object__populate_internal_map(): created map elements is destroyed upon close(map->fd).
- init_map_slots(): slots are freed after their initialization.

Quoted 3 lines
Cc Joe Stringer <[email protected]>
Signed-off-by Martynas Pumputis <[email protected]>

---

Applied to bpf-next, thanks.

Quoted 3 lines
 tools/lib/bpf/libbpf.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

[...]

CI Applied
#2 lore raw
In reply to Martynas Pumputis
This patch was applied to bpf/bpf-next.git (refs/heads/master):

Hello:

This patch was applied to bpf/bpf-next.git (refs/heads/master):

Quoted 11 lines · On Mon, 26 Jul 2021 17:20:01 +0200 you wrote:

When loading in parallel multiple programs which use the same to-be pinned map, it is possible that two instances of the loader will call bpf_object__create_maps() at the same time. If the map doesn't exist when both instances call bpf_object__reuse_map(), then one of the instances will fail with EEXIST when calling bpf_map__pin().

Fix the race by retrying reusing a map if bpf_map__pin() returns EEXIST. The fix is similar to the one in iproute2: e4c4685fd6e4 ("bpf:
Fix race condition with map pinning").

[...]

Here is the summary with links:
- [v2,bpf] libbpf: fix race when pinning maps in parallel

    https://git.kernel.org/bpf/bpf-next/c/043c5bb3c4f4

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html