Skip to content

Commit 12cc923

Browse files
rperierkees
authored andcommitted
tasklet: Introduce new initialization API
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 e99e88a ("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 <[email protected]> Co-developed-by: Allen Pais <[email protected]> Signed-off-by: Allen Pais <[email protected]> Reviewed-by: Greg Kroah-Hartman <[email protected]> Acked-by: Thomas Gleixner <[email protected]> Co-developed-by: Kees Cook <[email protected]> Signed-off-by: Kees Cook <[email protected]>
1 parent b13fecb commit 12cc923

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

include/linux/interrupt.h

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,9 @@ static inline struct task_struct *this_cpu_ksoftirqd(void)
585585

586586
/* Tasklets --- multithreaded analogue of BHs.
587587
588+
This API is deprecated. Please consider using threaded IRQs instead:
589+
https://lore.kernel.org/lkml/[email protected]
590+
588591
Main feature differing them of generic softirqs: tasklet
589592
is running only on one CPU simultaneously.
590593
@@ -608,10 +611,31 @@ struct tasklet_struct
608611
struct tasklet_struct *next;
609612
unsigned long state;
610613
atomic_t count;
611-
void (*func)(unsigned long);
614+
bool use_callback;
615+
union {
616+
void (*func)(unsigned long data);
617+
void (*callback)(struct tasklet_struct *t);
618+
};
612619
unsigned long data;
613620
};
614621

622+
#define DECLARE_TASKLET(name, _callback) \
623+
struct tasklet_struct name = { \
624+
.count = ATOMIC_INIT(0), \
625+
.callback = _callback, \
626+
.use_callback = true, \
627+
}
628+
629+
#define DECLARE_TASKLET_DISABLED(name, _callback) \
630+
struct tasklet_struct name = { \
631+
.count = ATOMIC_INIT(1), \
632+
.callback = _callback, \
633+
.use_callback = true, \
634+
}
635+
636+
#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \
637+
container_of(callback_tasklet, typeof(*var), tasklet_fieldname)
638+
615639
#define DECLARE_TASKLET_OLD(name, _func) \
616640
struct tasklet_struct name = { \
617641
.count = ATOMIC_INIT(0), \
@@ -691,6 +715,8 @@ extern void tasklet_kill(struct tasklet_struct *t);
691715
extern void tasklet_kill_immediate(struct tasklet_struct *t, unsigned int cpu);
692716
extern void tasklet_init(struct tasklet_struct *t,
693717
void (*func)(unsigned long), unsigned long data);
718+
extern void tasklet_setup(struct tasklet_struct *t,
719+
void (*callback)(struct tasklet_struct *));
694720

695721
/*
696722
* Autoprobing for irqs:

kernel/softirq.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,10 @@ static void tasklet_action_common(struct softirq_action *a,
547547
if (!test_and_clear_bit(TASKLET_STATE_SCHED,
548548
&t->state))
549549
BUG();
550-
t->func(t->data);
550+
if (t->use_callback)
551+
t->callback(t);
552+
else
553+
t->func(t->data);
551554
tasklet_unlock(t);
552555
continue;
553556
}
@@ -573,13 +576,26 @@ static __latent_entropy void tasklet_hi_action(struct softirq_action *a)
573576
tasklet_action_common(a, this_cpu_ptr(&tasklet_hi_vec), HI_SOFTIRQ);
574577
}
575578

579+
void tasklet_setup(struct tasklet_struct *t,
580+
void (*callback)(struct tasklet_struct *))
581+
{
582+
t->next = NULL;
583+
t->state = 0;
584+
atomic_set(&t->count, 0);
585+
t->callback = callback;
586+
t->use_callback = true;
587+
t->data = 0;
588+
}
589+
EXPORT_SYMBOL(tasklet_setup);
590+
576591
void tasklet_init(struct tasklet_struct *t,
577592
void (*func)(unsigned long), unsigned long data)
578593
{
579594
t->next = NULL;
580595
t->state = 0;
581596
atomic_set(&t->count, 0);
582597
t->func = func;
598+
t->use_callback = false;
583599
t->data = data;
584600
}
585601
EXPORT_SYMBOL(tasklet_init);

0 commit comments

Comments
 (0)