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]

Message-ID: <CAE-0n51KTiQjVqJgFe3S9qCiTM+2jdqyVZ1trNO1KanbQJccyA@mail.gmail.com>
Date:   Wed, 11 May 2022 13:02:09 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>
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

Quoting Thomas Gleixner (2022-05-10 02:20:01)
> On Wed, May 04 2022 at 15:31, Stephen Boyd wrote:
> > ---
> > 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.

Cool. Looks good to me. One problem 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;

This locates the correct address of the function pointer 'work.func' but
it needs to be dereferenced to return the function's address instead of
the pointer to the function. We don't really care about the function
signature so we could cast it to a void function pointer and deref:

                      void (**fn)(void) = addr + timer_hints[i].offset;

		      return *fn;

I'll send this version of the patch.