[PATCH pre-6.7] kprobes: Fix double free of kretprobe_holder

From: Petr Pavlu
Date: Thu Apr 04 2024 - 10:44:02 EST


When unregistering a kretprobe, the code in unregister_kretprobes() sets
rp->rph->rp to NULL which forces all associated kretprobe_instances
still in use to be later freed separately via free_rp_inst_rcu().

Function unregister_kretprobes() then calls free_rp_inst() which takes
care of releasing all currently unused kretprobe_instances, the ones
that are on the kretprobe's freelist. The code in free_rp_inst() counts
a number of these released kretprobe_instances and invokes
refcount_sub_and_test(count, &rp->rph->ref) to decrease the
kretprobe_holder's refcount and subsequently calls kfree(rp->rph) if the
function returns true, indicating the refcount reached zero.

It is possible that the number of released kretprobe_instances in
free_rp_inst() is zero and therefore refcount_sub_and_test() is invoked
with count=0. Additionally, depending on timing, it can happen
that all previously used kretprobe_instances were already freed via
free_rp_inst_rcu(). This means the refcount of kretprobe_holder already
reached zero and was deallocated.

The resulting call of refcount_sub_and_test(0, &rp->rph->ref) in
free_rp_inst() is then a use-after-free. If the memory previously
occupied by the refcount is still set to zero then the call returns true
and kretprobe_holder gets wrongly freed for the second time.

Fix the problem by adding a check for count>0 before calling
refcount_sub_and_test() in free_rp_inst().

Note that this code was reworked in v6.7 by commit 4bbd93455659
("kprobes: kretprobe scalability improvement") and the new objpool
implementation doesn't have this problem.

Fixes: d741bf41d7c7 ("kprobes: Remove kretprobe hash")
Signed-off-by: Petr Pavlu <petr.pavlu@xxxxxxxx>
---
kernel/kprobes.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0c6185aefaef..7ae5873545a1 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1942,10 +1942,9 @@ static inline void free_rp_inst(struct kretprobe *rp)
count++;
}

- if (refcount_sub_and_test(count, &rp->rph->ref)) {
+ if (count > 0 && refcount_sub_and_test(count, &rp->rph->ref))
kfree(rp->rph);
- rp->rph = NULL;
- }
+ rp->rph = NULL;
}

/* This assumes the 'tsk' is the current task or the is not running. */

base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
--
2.35.3