Because there is time gap between bpf_map_new_fd() and close_fd(), a
concurrent thread is able to close the new fd and opens a new, unrelated
file with the exact same fd number. Thereafter, this close_fd() might
inadvertently close the unrelated file.

To avoid such regression, drop close_fd() and override err when failed to
create map and failed to finalize the log.

In other word, when succeed in creating map but fail to finalize log,
users will get the map fd instead of the finalization error.

Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for 
map_create")
Signed-off-by: Leon Hwang <[email protected]>
---
 kernel/bpf/syscall.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 83de8fb9b9aa..322865a88b3a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t 
uattr, struct bpf_common_at
 
        /* preserve original error even if log finalization is successful */
        ret = bpf_log_attr_finalize(&attr_log, log);
-       if (ret) {
-               if (err >= 0)
-                       close_fd(err);
+       if (ret && err < 0)
+               /*
+                * Failed to finalize the log.
+                * Should not close_fd(err) here. Since the bpf_map_new_fd()
+                * has published the map fd, if a concurrent thread closes the
+                * fd, then opens new, unrelated file that receives the exact
+                * same fd number, close_fd(err) might inadvertently close the
+                * unrelated file.
+                * As a trade-off, override the err only when failed to finalize
+                * the log and failed to create map.
+                */
                err = ret;
-       }
 
        kfree(log);
        return err;
-- 
2.54.0


Reply via email to