bpf: sockmap, fix leaking maps with attached but not detached progs

"Linux Kernel Mailing List" <[email protected]>
Newsgroups gmane.linux.kernel.commits.head
Message-ID <[email protected]>
Web:        https://git.kernel.org/torvalds/c/3d9e952697de89b53227f06d4241f275eb99cfc4
Commit:     3d9e952697de89b53227f06d4241f275eb99cfc4
Parent:     1aa12bdf1bfb95db7e75bfecf0e39a65f4e8fbf8
Refname:    refs/heads/master
Author:     John Fastabend <[email protected]>
AuthorDate: Mon Feb 5 10:17:54 2018 -0800
Committer:  Daniel Borkmann <[email protected]>
CommitDate: Tue Feb 6 11:39:32 2018 +0100

    bpf: sockmap, fix leaking maps with attached but not detached progs
    
    When a program is attached to a map we increment the program refcnt
    to ensure that the program is not removed while it is potentially
    being referenced from sockmap side. However, if this same program
    also references the map (this is a reasonably common pattern in
    my programs) then the verifier will also increment the maps refcnt
    from the verifier. This is to ensure the map doesn't get garbage
    collected while the program has a reference to it.
    
    So we are left in a state where the map holds the refcnt on the
    program stopping it from being removed and releasing the map refcnt.
    And vice versa the program holds a refcnt on the map stopping it
    from releasing the refcnt on the prog.
    
    All this is fine as long as users detach the program while the
    map fd is still around. But, if the user omits this detach command
    we are left with a dangling map we can no longer release.
    
    To resolve this when the map fd is released decrement the program
    references and remove any reference from the map to the program.
    This fixes the issue with possibly dangling map and creates a
    user side API constraint. That is, the map fd must be held open
    for programs to be attached to a map.
    
    Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
    Signed-off-by: John Fastabend <[email protected]>
    Signed-off-by: Daniel Borkmann <[email protected]>
---
 kernel/bpf/sockmap.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index bd4a6d9c6709..48c33417d13c 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -620,11 +620,6 @@ static void sock_map_free(struct bpf_map *map)
 	}
 	rcu_read_unlock();
 
-	if (stab->bpf_verdict)
-		bpf_prog_put(stab->bpf_verdict);
-	if (stab->bpf_parse)
-		bpf_prog_put(stab->bpf_parse);
-
 	sock_map_remove_complete(stab);
 }
 
@@ -900,6 +895,19 @@ static int sock_map_update_elem(struct bpf_map *map,
 	return err;
 }
 
+static void sock_map_release(struct bpf_map *map, struct file *map_file)
+{
+	struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
+	struct bpf_prog *orig;
+
+	orig = xchg(&stab->bpf_parse, NULL);
+	if (orig)
+		bpf_prog_put(orig);
+	orig = xchg(&stab->bpf_verdict, NULL);
+	if (orig)
+		bpf_prog_put(orig);
+}
+
 const struct bpf_map_ops sock_map_ops = {
 	.map_alloc = sock_map_alloc,
 	.map_free = sock_map_free,
@@ -907,6 +915,7 @@ const struct bpf_map_ops sock_map_ops = {
 	.map_get_next_key = sock_map_get_next_key,
 	.map_update_elem = sock_map_update_elem,
 	.map_delete_elem = sock_map_delete_elem,
+	.map_release = sock_map_release,
 };
 
 BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
--
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.