[PATCH v2 bpf] libbpf: fix race when pinning maps in parallel

[PATCH v2 bpf] libbpf: fix race when pinning maps in parallel

[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.

Cc: Joe Stringer <joe@wand.net.nz>
Signed-off-by: Martynas Pumputis <m@lambda.lt>
---
 tools/lib/bpf/libbpf.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6f5e2757bb3c..9a39453bb45b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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;
 			}
 		}
-- 
2.32.0

Re: [PATCH v2 bpf] libbpf: fix race when pinning maps in parallel

[Andrii Nakryiko]

On Mon, Jul 26, 2021 at 8:19 AM Martynas Pumputis <m@lambda.lt> 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").
>
> 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 <joe@wand.net.nz>
> Signed-off-by: Martynas Pumputis <m@lambda.lt>
> ---

Applied to bpf-next, thanks.

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

[...]

Re: [PATCH v2 bpf] libbpf: fix race when pinning maps in parallel

[patchwork-bot+netdevbpf]

Hello:

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

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