On Tue, Jul 21, 2020 at 8:23 PM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > On Sun, Jul 19, 2020 at 6:35 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > > > In preparation to support a new policy format with a more optimal > > representation of filename transition rules, this patch applies an > > equivalent change from kernel commit c3a276111ea2 ("selinux: optimize > > storage of filename transitions"). > > > > See the kernel commit's description [1] for the rationale behind this > > representation. This change doesn't bring any measurable difference of > > policy build performance (semodule -B) on Fedora. > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?id=c3a276111ea2572399281988b3129683e2a6b60b > > > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > > --- > > > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c > > index c6733fa4..01a90438 100644 > > --- a/checkpolicy/policy_define.c > > +++ b/checkpolicy/policy_define.c > > @@ -3388,40 +3387,21 @@ int define_filename_trans(void) > <snip> > > + dup_name = NULL; > > + rc = policydb_filetrans_insert( > > + policydbp, s+1, t+1, c+1, name, > > + &dup_name, otype, NULL > > + ); > > + free(dup_name); > > The dup_name / name_alloc handling seems rather odd. In every caller > except one you follow the pattern above, where we set it to NULL, call > policydb_filetrans_insert, and then free it immediately. I'm not sure > why you are doing it this way. The original intent was to allow a choice between putting the already allocated string into the key (when the caller function owns it) vs. allocating a new string (when the caller gets just a read-only reference to it), while also leaving the string allocated in case such key already exists (so that the caller can use it in an error message, as filename_trans_read_one_compat() used to do). However, after rebasing on top of d27aa22dbeec ("libsepol: drop broken warning on duplicate filename transitions") such function interface seems to be no longer optimal... I'll see if I can simplify it. Thanks for pointing it out. -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.