Patch "bpf: put bpf_link's program when link is safe to be deallocated" has been added to the 6.6-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    bpf: put bpf_link's program when link is safe to be deallocated

to the 6.6-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     bpf-put-bpf_link-s-program-when-link-is-safe-to-be-d.patch
and it can be found in the queue-6.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 665ca79fe5cc241071b012033d3cb573e941f76f
Author: Andrii Nakryiko <andrii@xxxxxxxxxx>
Date:   Fri Nov 1 11:17:52 2024 -0700

    bpf: put bpf_link's program when link is safe to be deallocated
    
    [ Upstream commit f44ec8733a8469143fde1984b5e6931b2e2f6f3f ]
    
    In general, BPF link's underlying BPF program should be considered to be
    reachable through attach hook -> link -> prog chain, and, pessimistically,
    we have to assume that as long as link's memory is not safe to free,
    attach hook's code might hold a pointer to BPF program and use it.
    
    As such, it's not (generally) correct to put link's program early before
    waiting for RCU GPs to go through. More eager bpf_prog_put() that we
    currently do is mostly correct due to BPF program's release code doing
    similar RCU GP waiting, but as will be shown in the following patches,
    BPF program can be non-sleepable (and, thus, reliant on only "classic"
    RCU GP), while BPF link's attach hook can have sleepable semantics and
    needs to be protected by RCU Tasks Trace, and for such cases BPF link
    has to go through RCU Tasks Trace + "classic" RCU GPs before being
    deallocated. And so, if we put BPF program early, we might free BPF
    program before we free BPF link, leading to use-after-free situation.
    
    So, this patch defers bpf_prog_put() until we are ready to perform
    bpf_link's deallocation. At worst, this delays BPF program freeing by
    one extra RCU GP, but that seems completely acceptable. Alternatively,
    we'd need more elaborate ways to determine BPF hook, BPF link, and BPF
    program lifetimes, and how they relate to each other, which seems like
    an unnecessary complication.
    
    Note, for most BPF links we still will perform eager bpf_prog_put() and
    link dealloc, so for those BPF links there are no observable changes
    whatsoever. Only BPF links that use deferred dealloc might notice
    slightly delayed freeing of BPF programs.
    
    Also, to reduce code and logic duplication, extract program put + link
    dealloc logic into bpf_link_dealloc() helper.
    
    Link: https://lore.kernel.org/20241101181754.782341-1-andrii@xxxxxxxxxx
    Tested-by: Jordan Rife <jrife@xxxxxxxxxx>
    Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
    Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 252aed82d45ea..ba38c08a9a059 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2870,12 +2870,24 @@ void bpf_link_inc(struct bpf_link *link)
 	atomic64_inc(&link->refcnt);
 }
 
+static void bpf_link_dealloc(struct bpf_link *link)
+{
+	/* now that we know that bpf_link itself can't be reached, put underlying BPF program */
+	if (link->prog)
+		bpf_prog_put(link->prog);
+
+	/* free bpf_link and its containing memory */
+	if (link->ops->dealloc_deferred)
+		link->ops->dealloc_deferred(link);
+	else
+		link->ops->dealloc(link);
+}
+
 static void bpf_link_defer_dealloc_rcu_gp(struct rcu_head *rcu)
 {
 	struct bpf_link *link = container_of(rcu, struct bpf_link, rcu);
 
-	/* free bpf_link and its containing memory */
-	link->ops->dealloc_deferred(link);
+	bpf_link_dealloc(link);
 }
 
 static void bpf_link_defer_dealloc_mult_rcu_gp(struct rcu_head *rcu)
@@ -2897,7 +2909,6 @@ static void bpf_link_free(struct bpf_link *link)
 		sleepable = link->prog->aux->sleepable;
 		/* detach BPF program, clean up used resources */
 		ops->release(link);
-		bpf_prog_put(link->prog);
 	}
 	if (ops->dealloc_deferred) {
 		/* schedule BPF link deallocation; if underlying BPF program
@@ -2908,8 +2919,9 @@ static void bpf_link_free(struct bpf_link *link)
 			call_rcu_tasks_trace(&link->rcu, bpf_link_defer_dealloc_mult_rcu_gp);
 		else
 			call_rcu(&link->rcu, bpf_link_defer_dealloc_rcu_gp);
-	} else if (ops->dealloc)
-		ops->dealloc(link);
+	} else if (ops->dealloc) {
+		bpf_link_dealloc(link);
+	}
 }
 
 static void bpf_link_put_deferred(struct work_struct *work)




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux