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);elset->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.