On Wed, May 18, 2022 at 12:03 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > 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. I just merged this into my local selinux/next branch and I'll be pushing it up to the mail selinux tree later today - thanks! -- paul-moore.com
