linux-kernel - Re: [PATCH] timers: Provide a better debugobjects hint for delayed works

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 May 2022 11:20:01 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Stephen Boyd <swboyd@...omium.org>,
        John Stultz <john.stultz@...aro.org>
Cc:     linux-kernel@...r.kernel.org, patches@...ts.linux.dev,
        Tejun Heo <tj@...nel.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Guenter Roeck <groeck@...omium.org>
Subject: Re: [PATCH] timers: Provide a better debugobjects hint for delayed
 works

On Wed, May 04 2022 at 15:31, Stephen Boyd wrote:
> Provide better information here by special casing delayed works in the
> timer debugobjects hint logic so that the work function is returned
> instead of the timer function delayed_work_timer_fn(). This will help us
> understand what delayed work was pending that got freed, leading to
> faster bug resolutions.

Makes sense.

> ---
> I have an alternative approach which is to treat delayed works with a
> different debug_obj_descr structure but it basically boils down to
> another version of timer debugobjects in the workqueue code. The idea is
> to make the delayed work active once the timer is queued and then
> convert it over from a delayed work descriptor to a work descriptor once
> the timer runs delayed_work_timer_fn() or when we pull it off to flush
> out.

Nah.

>  #include <linux/uaccess.h>
>  #include <asm/unistd.h>
> @@ -617,7 +618,17 @@ static const struct debug_obj_descr timer_debug_descr;
>  
>  static void *timer_debug_hint(void *addr)
>  {
> -	return ((struct timer_list *) addr)->function;
> +	struct timer_list *timer = addr;
> +
> +	if (timer->function == delayed_work_timer_fn) {
> +		struct delayed_work *dwork;
> +
> +		dwork = container_of(timer, struct delayed_work, timer);
> +
> +		return dwork->work.func;
> +	}

The same issue exists for kthread_delayed_work_timer_fn.

So maybe something like the uncompiled/untested below.

Thanks,

        tglx
---
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -638,9 +638,35 @@ static void internal_add_timer(struct ti
 
 static const struct debug_obj_descr timer_debug_descr;
 
+struct timer_hint {
+	void	(*function)(struct timer_list *);
+	long	offset;
+};
+
+#define TIMER_HINT(fn, container, timr, hintfn)			\
+	{							\
+		.function = fn,					\
+		.offset	  = offsetof(container, hintfn) -	\
+			    offsetof(container, timr)	\
+	}
+
+static const struct timer_hint timer_hints[] = {
+	TIMER_HINT(delayed_work_timer_fn,
+		   struct delayed_work, timer, work.func),
+	TIMER_HINT(kthread_delayed_work_timer_fn,
+		   struct kthread_delayed_work, timer, work.func),
+};
+
 static void *timer_debug_hint(void *addr)
 {
-	return ((struct timer_list *) addr)->function;
+	struct timer_list *timer = addr;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(timer_hints); i++) {
+		if (timer_hints[i].function == timer->function)
+			return addr + timer_hints[i].offset;
+	}
+	return timer->function;
 }
 
 static bool timer_is_static_object(void *addr)