From: Kees Cook <keescook@chromium.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
Oscar Carter <oscar.carter@gmx.com>,
Mitchell Blank Jr <mitch@sfgoth.com>,
kernel-hardening@lists.openwall.com,
Peter Zijlstra <peterz@infradead.org>,
kgdb-bugreport@lists.sourceforge.net,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
alsa-devel@alsa-project.org, Allen Pais <allen.lkml@gmail.com>,
Christian Gromm <christian.gromm@microchip.com>,
Will Deacon <will@kernel.org>,
devel@driverdev.osuosl.org, Jonathan Corbet <corbet@lwn.net>,
Daniel Thompson <daniel.thompson@linaro.org>,
"David S. Miller" <davem@davemloft.net>,
Masahiro Yamada <masahiroy@kernel.org>,
Takashi Iwai <tiwai@suse.com>,
Julian Wiedmann <jwi@linux.ibm.com>,
"Matthew Wilcox \(Oracle\)" <willy@infradead.org>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Nishka Dasgupta <nishkadg.linux@gmail.com>,
Jiri Slaby <jslaby@suse.com>, Jakub Kicinski <kuba@kernel.org>,
Guenter Roeck <linux@roeck-us.net>,
Wambui Karuga <wambui.karugax@gmail.com>,
Kees Cook <keescook@chromium.org>,
Vasily Gorbik <gor@linux.ibm.com>,
linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
Heiko Carstens <hca@linux.ibm.com>,
linux-input@vger.kernel.org, Ursula Braun <ubraun@linux.ibm.com>,
Stephen Boyd <swboyd@chromium.org>,
Chris Packham <chris.packham@alliedtelesis.co.nz>,
Harald Freudenberger <freude@linux.ibm.com>,
Thomas Gleixner <tglx@linutronix.de>,
Felipe Balbi <balbi@kernel.org>, Kyungtae Kim <kt0755@gmail.com>,
netdev@vger.kernel.org,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Douglas Anderson <dianders@chromium.org>,
Kevin Curtis <kevin.curtis@farsite.co.uk>,
linux-usb@vger.kernel.org,
Jason Wessel <jason.wessel@windriver.com>,
Romain Perier <romain.perier@gmail.com>,
Karsten Graul <kgraul@linux.ibm.com>
Subject: [PATCH 3/3] tasklet: Introduce new initialization API
Date: Wed, 15 Jul 2020 20:08:47 -0700 [thread overview]
Message-ID: <20200716030847.1564131-4-keescook@chromium.org> (raw)
In-Reply-To: <20200716030847.1564131-1-keescook@chromium.org>
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(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index b911196f03eb..15570b237e53 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -608,10 +608,30 @@ struct tasklet_struct
struct tasklet_struct *next;
unsigned long state;
atomic_t count;
- void (*func)(unsigned long);
+ bool use_callback;
+ union {
+ void (*func)(unsigned long data);
+ void (*callback)(struct tasklet_struct *t);
+ };
unsigned long data;
};
+#define DECLARE_TASKLET(name, _callback) \
+struct tasklet_struct name = { \
+ .count = ATOMIC_INIT(0), \
+ .callback = _callback, \
+ .use_callback = true, \
+}
+
+#define DECLARE_TASKLET_DISABLED(name, _callback) \
+struct tasklet_struct name = { \
+ .count = ATOMIC_INIT(1), \
+ .callback = _callback, \
+}
+
+#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \
+ container_of(callback_tasklet, typeof(*var), tasklet_fieldname)
+
#define DECLARE_TASKLET_OLD(name, _func) \
struct tasklet_struct name = { \
.count = ATOMIC_INIT(0), \
@@ -691,6 +711,8 @@ extern void tasklet_kill(struct tasklet_struct *t);
extern void tasklet_kill_immediate(struct tasklet_struct *t, unsigned int cpu);
extern void tasklet_init(struct tasklet_struct *t,
void (*func)(unsigned long), unsigned long data);
+extern void tasklet_setup(struct tasklet_struct *t,
+ void (*callback)(struct tasklet_struct *));
/*
* Autoprobing for irqs:
diff --git a/kernel/softirq.c b/kernel/softirq.c
index c4201b7f42b1..292e7c2d2333 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -547,7 +547,10 @@ static void tasklet_action_common(struct softirq_action *a,
if (!test_and_clear_bit(TASKLET_STATE_SCHED,
&t->state))
BUG();
- t->func(t->data);
+ if (t->use_callback)
+ t->callback(t);
+ else
+ t->func(t->data);
tasklet_unlock(t);
continue;
}
@@ -573,6 +576,18 @@ static __latent_entropy void tasklet_hi_action(struct softirq_action *a)
tasklet_action_common(a, this_cpu_ptr(&tasklet_hi_vec), HI_SOFTIRQ);
}
+void tasklet_setup(struct tasklet_struct *t,
+ void (*callback)(struct tasklet_struct *))
+{
+ t->next = NULL;
+ t->state = 0;
+ atomic_set(&t->count, 0);
+ t->callback = callback;
+ t->use_callback = true;
+ t->data = 0;
+}
+EXPORT_SYMBOL(tasklet_setup);
+
void tasklet_init(struct tasklet_struct *t,
void (*func)(unsigned long), unsigned long data)
{
@@ -580,6 +595,7 @@ void tasklet_init(struct tasklet_struct *t,
t->state = 0;
atomic_set(&t->count, 0);
t->func = func;
+ t->use_callback = false;
t->data = data;
}
EXPORT_SYMBOL(tasklet_init);
--
2.25.1
next prev parent reply other threads:[~2020-07-16 7:22 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-16 3:08 [PATCH 0/3] Modernize tasklet callback API Kees Cook
2020-07-16 3:08 ` [PATCH 1/3] usb: gadget: udc: Avoid tasklet passing a global Kees Cook
2020-07-16 7:28 ` Greg Kroah-Hartman
2020-07-16 19:41 ` Kees Cook
2020-07-31 9:20 ` Felipe Balbi
2020-07-16 3:08 ` [PATCH 2/3] treewide: Replace DECLARE_TASKLET() with DECLARE_TASKLET_OLD() Kees Cook
2020-07-16 7:30 ` Greg Kroah-Hartman
2020-07-16 11:29 ` Matthew Wilcox
2020-07-16 19:15 ` Kees Cook
2020-07-16 3:08 ` Kees Cook [this message]
2020-07-16 7:30 ` [PATCH 3/3] tasklet: Introduce new initialization API Greg Kroah-Hartman
2020-07-16 15:37 ` Matthew Wilcox
2020-07-16 19:22 ` Kees Cook
2020-07-16 7:57 ` [PATCH 0/3] Modernize tasklet callback API Peter Zijlstra
2020-07-16 8:15 ` Sebastian Andrzej Siewior
2020-07-16 19:24 ` Kees Cook
2020-07-16 19:14 ` Kees Cook
2020-07-16 20:48 ` Dmitry Torokhov
2020-07-16 21:24 ` Kees Cook
2020-07-30 7:03 ` Thomas Gleixner
2020-07-30 18:14 ` Kees Cook
2020-08-03 8:46 ` Allen
2020-08-11 12:16 ` Allen
2020-08-11 21:33 ` Kees Cook
2020-08-12 6:21 ` Takashi Iwai
2020-08-12 11:32 ` Allen
2020-08-12 12:31 ` Allen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200716030847.1564131-4-keescook@chromium.org \
--to=keescook@chromium.org \
--cc=allen.lkml@gmail.com \
--cc=alsa-devel@alsa-project.org \
--cc=balbi@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=borntraeger@de.ibm.com \
--cc=chris.packham@alliedtelesis.co.nz \
--cc=christian.gromm@microchip.com \
--cc=corbet@lwn.net \
--cc=daniel.thompson@linaro.org \
--cc=davem@davemloft.net \
--cc=devel@driverdev.osuosl.org \
--cc=dianders@chromium.org \
--cc=dmitry.torokhov@gmail.com \
--cc=freude@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=gregkh@linuxfoundation.org \
--cc=hca@linux.ibm.com \
--cc=jason.wessel@windriver.com \
--cc=jslaby@suse.com \
--cc=jwi@linux.ibm.com \
--cc=kernel-hardening@lists.openwall.com \
--cc=kevin.curtis@farsite.co.uk \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=kgraul@linux.ibm.com \
--cc=kt0755@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=masahiroy@kernel.org \
--cc=mitch@sfgoth.com \
--cc=netdev@vger.kernel.org \
--cc=nishkadg.linux@gmail.com \
--cc=oscar.carter@gmx.com \
--cc=peterz@infradead.org \
--cc=rafael.j.wysocki@intel.com \
--cc=romain.perier@gmail.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=swboyd@chromium.org \
--cc=tglx@linutronix.de \
--cc=tiwai@suse.com \
--cc=ubraun@linux.ibm.com \
--cc=wambui.karugax@gmail.com \
--cc=will@kernel.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).