|
Message-ID: <202007161216.9C9784FEBE@keescook> Date: Thu, 16 Jul 2020 12:22:17 -0700 From: Kees Cook <keescook@...omium.org> To: Matthew Wilcox <willy@...radead.org> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Romain Perier <romain.perier@...il.com>, Allen Pais <allen.lkml@...il.com>, Thomas Gleixner <tglx@...utronix.de>, Oscar Carter <oscar.carter@....com>, Dmitry Torokhov <dmitry.torokhov@...il.com>, Kevin Curtis <kevin.curtis@...site.co.uk>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Harald Freudenberger <freude@...ux.ibm.com>, Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ibm.com>, Jiri Slaby <jslaby@...e.com>, Felipe Balbi <balbi@...nel.org>, Jason Wessel <jason.wessel@...driver.com>, Daniel Thompson <daniel.thompson@...aro.org>, Douglas Anderson <dianders@...omium.org>, Mitchell Blank Jr <mitch@...oth.com>, Julian Wiedmann <jwi@...ux.ibm.com>, Karsten Graul <kgraul@...ux.ibm.com>, Ursula Braun <ubraun@...ux.ibm.com>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Christian Gromm <christian.gromm@...rochip.com>, Nishka Dasgupta <nishkadg.linux@...il.com>, Masahiro Yamada <masahiroy@...nel.org>, Stephen Boyd <swboyd@...omium.org>, Wambui Karuga <wambui.karugax@...il.com>, Guenter Roeck <linux@...ck-us.net>, Chris Packham <chris.packham@...iedtelesis.co.nz>, Kyungtae Kim <kt0755@...il.com>, Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>, "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>, Jonathan Corbet <corbet@....net>, Peter Zijlstra <peterz@...radead.org>, Will Deacon <will@...nel.org>, linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, linux-s390@...r.kernel.org, devel@...verdev.osuosl.org, linux-usb@...r.kernel.org, kgdb-bugreport@...ts.sourceforge.net, alsa-devel@...a-project.org, kernel-hardening@...ts.openwall.com Subject: Re: [PATCH 3/3] tasklet: Introduce new initialization API On Thu, Jul 16, 2020 at 04:37:04PM +0100, Matthew Wilcox wrote: > On Wed, Jul 15, 2020 at 08:08:47PM -0700, Kees Cook wrote: > > +#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, \ > > +} > > You forgot to set use_callback here. Eek; thank you. > > @@ -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); > > I think this is the wrong way to do the conversion. Start out by setting > t->data to (unsigned long)t in the new initialisers. Then convert the > drivers (all 350 of them) to the new API. Then you can get rid of 'data' > from the tasklet_struct. That's what I did when I converted timer_struct, and it ended up creating a mess for Control Flow Integrity checking. (The problem isn't actually casting .data, but rather in how the callsite calls the callback -- casting the callback assignments doesn't fix the mismatch between the caller and the callback's expectation about the function prototype under CFI.) I got lucky with timer_struct (in v4.14) in that not much had been converted, and I was able to do the entire conversion in the next kernel release. So, this time, I'm trying to avoid the prototype mismatch mess by providing a selector to determine which prototype the callback should be called through, and I was happy to discover I could do it without growing the tasklet structure. Obviously the memory corruption safety improvement won't be realized until both .data, .use_callback, and .func are removed, but that was true even with the earlier style of conversion. -- Kees Cook
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.