Kees Cook <[email protected]> writes: > On Tue, Sep 08, 2020 at 12:59:49AM -0400, Gabriel Krisman Bertazi wrote: >> Christian Brauner <[email protected]> writes: >> >> > On Fri, Sep 04, 2020 at 04:31:39PM -0400, Gabriel Krisman Bertazi wrote: >> >> index afe01e232935..3511c98a7849 100644 >> >> --- a/include/linux/sched.h >> >> +++ b/include/linux/sched.h >> >> @@ -959,7 +959,11 @@ struct task_struct { >> >> kuid_t loginuid; >> >> unsigned int sessionid; >> >> #endif >> >> - struct seccomp seccomp; >> >> + >> >> + struct { >> >> + unsigned int syscall_intercept; >> >> + struct seccomp seccomp; >> >> + }; >> > >> > If there's no specific reason to do this I'd not wrap this in an >> > anonymous struct. It doesn't really buy anything and there doesn't seem >> > to be precedent in struct task_struct right now. Also, if this somehow >> > adds padding it seems you might end up increasing the size of struct >> > task_struct more than necessary by accident? (I might be wrong >> > though.) >> >> Hi Christian, >> >> Thanks for your review on this and on the other patches of this series. >> >> I wrapped these to prevent struct layout randomization from separating >> the flags field from seccomp, as they are going to be used together and >> I was trying to reduce overhead to seccomp entry due to two cache misses >> when reading this structure. Measuring it seccomp_benchmark didn't show >> any difference with the unwrapped version, so perhaps it was a bit of >> premature optimization? > > That should not be a thing to think about here. Structure randomization > already has a mode to protect against cache line issues. I would leave > this as just a new member; no wrapping struct.
Makes sense. I will drop it for the next iteration. Thanks! -- Gabriel Krisman Bertazi
