On Wed, May 18, 2022 at 2:53 AM Gong Ruiqi <gongruiqi1@xxxxxxxxxx> wrote: > On 2022/05/18 9:39, Paul Moore wrote: > > On Tue, May 17, 2022 at 9:21 PM GONG, Ruiqi <gongruiqi1@xxxxxxxxxx> wrote: > >> > >> Randomize the layout of struct selinux_audit_data as suggested in [1], > >> since it contains a pointer to struct selinux_state, an already > >> randomized strucure. > >> > >> [1]: https://github.com/KSPP/linux/issues/188 > >> > >> Signed-off-by: GONG, Ruiqi <gongruiqi1@xxxxxxxxxx> > >> --- > >> security/selinux/include/avc.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h > >> index 2b372f98f2d7..5525b94fd266 100644 > >> --- a/security/selinux/include/avc.h > >> +++ b/security/selinux/include/avc.h > >> @@ -53,7 +53,7 @@ struct selinux_audit_data { > >> u32 denied; > >> int result; > >> struct selinux_state *state; > >> -}; > >> +} __randomize_layout; > > > > I'll apologize in advance for the stupid question, but does > > Not at all :) > > > __randomize_layout result in any problems when the struct is used in a > > trace event? (see include/trace/events/avc.h) > > No, as least it doesn't in the testing I did. I believe we can use the > struct tagged with __randomize_layout as normal except that 1) it should > be initialized with a designated initializer, and 2) pointers to this > type can't be cast to/from pointers to another type. Other operations > like dereferencing members of the struct (as in > include/trace/events/avc.h) shouldn't be a problem. > > I did a testing to the patch on a qemu vm by running the selinux > testsuite with tracing events "avc:selinux_audited" enabled. The > testsuite completed successfully and from the tracing log I saw nothing > abnormal with my bare eyes. You can do more testing if you want or you > have other ideas of how to do so ;) That's great, thanks for verifying this. I was aware of the other restrictions but wasn't sure about tracing. Now I know :) It's too late to go into selinux/next for this dev cycle, but I'll queue this up for selinux/next once the merge window closes. Thanks again! -- paul-moore.com
