Re: [PATCH 3/3] tasklet: Introduce new initialization API

Greg Kroah-Hartman
2020-07-16T07:30:35Z

On Wed, Jul 15, 2020 at 08:08:47PM -0700, Kees Cook wrote:
> From: Romain Perier <romain.perier@gmail.com>
>
> Nowadays, modern kernel subsystems that use callbacks pass the data
> structure associated with a given callback as argument to the callback.
> The tasklet subsystem remains one which passes an arbitrary unsigned
> long to the callback function. This has several problems:
>
> - This keeps an extra field for storing the argument in each tasklet
> data structure, it bloats the tasklet_struct structure with a redundant
> .data field
>
> - No type checking can be performed on this argument. Instead of
> using container_of() like other callback subsystems, it forces callbacks
> to do explicit type cast of the unsigned long argument into the required
> object type.
>
> - Buffer overflows can overwrite the .func and the .data field, so
> an attacker can easily overwrite the function and its first argument
> to whatever it wants.
>
> Add a new tasklet initialization API, via DECLARE_TASKLET() and
> tasklet_setup(), which will replace the existing ones.
>
> This work is greatly inspired by the timer_struct conversion series,
> see commit e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
>
> To avoid problems with both -Wcast-function-type (which is enabled in
> the kernel via -Wextra is several subsystems), and with mismatched
> function prototypes when build with Control Flow Integrity enabled,
> this adds the "use_callback" member to let the tasklet caller choose
> which union member to call through. Once all old API uses are removed,
> this and the .data member will be removed as well. (On 64-bit this does
> not grow the struct size as the new member fills the hole after atomic_t,
> which is also "int" sized.)
>
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> Co-developed-by: Allen Pais <allen.lkml@gmail.com>
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> Co-developed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> include/linux/interrupt.h | 24 +++++++++++++++++++++++-
> kernel/softirq.c | 18 +++++++++++++++++-
> 2 files changed, 40 insertions(+), 2 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>