Commit f91f7ac9 authored by Petr Pavlu's avatar Petr Pavlu Committed by Kees Cook
Browse files

refcount: Report UAF for refcount_sub_and_test(0) when counter==0 



When a reference counter is at zero and refcount_sub_and_test() is invoked
to subtract zero, the function accepts this request without any warning and
returns true. This behavior does not seem ideal because the counter being
already at zero indicates a use-after-free. Furthermore, returning true by
refcount_sub_and_test() in this case potentially results in a double-free
done by its caller.

Modify the underlying function __refcount_sub_and_test() to warn about this
case as a use-after-free and have it return false to avoid the potential
double-free.

Signed-off-by: default avatarPetr Pavlu <petr.pavlu@suse.com>
Acked-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20240717130023.5675-1-petr.pavlu@suse.com


Signed-off-by: default avatarKees Cook <kees@kernel.org>
parent de9c2c66

drivers/misc/lkdtm/refcount.c

+16 −0
Original line number Diff line number Diff line
@@ -182,6 +182,21 @@ static void lkdtm_REFCOUNT_SUB_AND_TEST_NEGATIVE(void) 
	check_negative(&neg, 3);

}



/*

 * A refcount_sub_and_test() by zero when the counter is at zero should act like

 * refcount_sub_and_test() above when going negative.

 */

static void lkdtm_REFCOUNT_SUB_AND_TEST_ZERO(void)

{

	refcount_t neg = REFCOUNT_INIT(0);



	pr_info("attempting bad refcount_sub_and_test() at zero\n");

	if (refcount_sub_and_test(0, &neg))

		pr_warn("Weird: refcount_sub_and_test() reported zero\n");



	check_negative(&neg, 0);

}



static void check_from_zero(refcount_t *ref)

{

	switch (refcount_read(ref)) {
@@ -400,6 +415,7 @@ static struct crashtype crashtypes[] = { 
	CRASHTYPE(REFCOUNT_DEC_NEGATIVE),

	CRASHTYPE(REFCOUNT_DEC_AND_TEST_NEGATIVE),

	CRASHTYPE(REFCOUNT_SUB_AND_TEST_NEGATIVE),

	CRASHTYPE(REFCOUNT_SUB_AND_TEST_ZERO),

	CRASHTYPE(REFCOUNT_INC_ZERO),

	CRASHTYPE(REFCOUNT_ADD_ZERO),

	CRASHTYPE(REFCOUNT_INC_SATURATED),

include/linux/refcount.h

+2 −2
Original line number Diff line number Diff line
@@ -266,12 +266,12 @@ bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp) 
	if (oldp)

		*oldp = old;



	if (old == i) {

	if (old > 0 && old == i) {

		smp_acquire__after_ctrl_dep();

		return true;

	}



	if (unlikely(old < 0 || old - i < 0))

	if (unlikely(old <= 0 || old - i < 0))

		refcount_warn_saturate(r, REFCOUNT_SUB_UAF);



	return false;