| Message ID | 20220420004241.2093-11-joao@overdrivepizza.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Kernel FineIBT Support | expand |
On Tue, Apr 19, 2022 at 05:42:40PM -0700, joao@overdrivepizza.com wrote: > Unions will make function pointers with different prototypes be used through > the same call. This leads into the same call instruction being used for > calling functions with different prototypes, making them unsuitable for > prototype-based fine-grained CFI. Why? Shouldn't the callers be using different prototypes? > Fix this CFI policy violation by removing the function pointer union in > the tasklet struct. The good news is that tasklet is on the way out the door[1], so this may quickly become a non-issue, but also to that end, this fix is hardly a problem for a deprecated API... -Kees [1] https://lore.kernel.org/linux-hardening/20220419211658.11403-1-apais@linux.microsoft.com/ > > Signed-off-by: Joao Moreira <joao@overdrivepizza.com> > --- > include/linux/interrupt.h | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index f40754caaefa..8d5504b0f20b 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -650,10 +650,8 @@ struct tasklet_struct > unsigned long state; > atomic_t count; > bool use_callback; > - union { > - void (*func)(unsigned long data); > - void (*callback)(struct tasklet_struct *t); > - }; > + void (*func)(unsigned long data); > + void (*callback)(struct tasklet_struct *t); > unsigned long data; > }; > > -- > 2.35.1 >
>> Fix this CFI policy violation by removing the function pointer union >> in >> the tasklet struct. > > The good news is that tasklet is on the way out the door[1], so this > may > quickly become a non-issue, but also to that end, this fix is hardly a > problem for a deprecated API... You are right, sorry for the noise. I looked a bit further and the problem I saw was actually caused by a compiler bug fusing similar instructions/basic blocks. It was fixed when I later stumbled on the problem again and added the following lines (668 and 669 in llvm/lib/CodeGen/MachineInstr.cpp) to the compiler, but without properly realizing what was actually behind the previous issue. Hopefully this is at least a good heads-up about possible pitfalls to other people (@Sami) implementing CFI in the compiler. https://github.com/lvwr/llvm-project/commit/0a22ca42877fd156ce95145b11f29c642092dbb7#diff-92843a1f037a9a1e56f92242c4e1746a1166a6b7044ad47a0b4fd2f4b1c6a359R668-R669
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index f40754caaefa..8d5504b0f20b 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -650,10 +650,8 @@ struct tasklet_struct unsigned long state; atomic_t count; bool use_callback; - union { - void (*func)(unsigned long data); - void (*callback)(struct tasklet_struct *t); - }; + void (*func)(unsigned long data); + void (*callback)(struct tasklet_struct *t); unsigned long data; };