On Fri, Jun 2, 2023 at 3:13 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> On Wed, 31 May 2023 at 13:32, Juraj Marcin <juraj@xxxxxxxxxxxxxxx> wrote:
> >
> > Currently, filename transitions are stored separately from other type
> > enforcement rules. This leads to possibly sub-optimal performance and
> > makes further improvements cumbersome.
> >
> > This patch adds a symbol table of filename transitions to the transition
> > structure added to avtab in the previous patch. It also implements
> > functions required for reading and writing filename transitions in
> > kernel policy format and updates computation of new type to use filename
> > transitions embedded in avtab. Last but not least, it updates the
> > conflict check in the conditional avtab to account for empty transitions
> > in the non-conditional avtab.
> >
> > These changes are expected to cause higher memory usage, as now there
> > needs to be a filename transition structure for every stype. This patch
> > effectively undoes most of the commit c3a276111ea2 ("selinux: optimize
> > storage of filename transitions"), but this will be mitigated by
> > providing support for matching prefix/suffix of the filename for
> > filename transitions in future patches which will reduce to need to have
> > so many of them.
> >
> > On the other hand, the changes do not significantly slow down the
> > creation of new files.
> >
> > Kernel | Mem | Create test_tty | Create test_tty | osbench [1]
> > | usage | (real time) | (kernel time) | create_files
> > -----------+-------+-----------------+-----------------+--------------
> > reference | 155MB | 1.3440 ms/file | 1.0071 ms/file | 10.6507 us/file
> > this patch | 198MB | 1.3912 ms/file | 1.0172 ms/file | 10.5567 us/file
> >
> > Create test_tty benchmark:
> >
> > mknod /dev/test_tty c 4 1
> > time for i in `seq 1 10000`; do
> > mknod /dev/test_tty$i c 4 1
> > done
> >
> > This benchmark should simulate the worst case scenario as many filename
> > transitions affect files created in the /dev directory.
> >
> > [1] https://github.com/mbitsnbites/osbench
> >
> > Reviewed-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > Signed-off-by: Juraj Marcin <juraj@xxxxxxxxxxxxxxx>
> > ---
<snip>
> > diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
> > index 6c8eb7c379cf..162ef1be85e7 100644
> > --- a/security/selinux/ss/avtab.h
> > +++ b/security/selinux/ss/avtab.h
> > @@ -22,6 +22,7 @@
> > #define _SS_AVTAB_H_
> >
> > #include "security.h"
> > +#include "symtab.h"
> >
> > struct avtab_key {
> > u16 source_type; /* source type */
> > @@ -49,6 +50,7 @@ struct avtab_key {
> >
> > struct avtab_trans {
> > u32 otype; /* default resulting type of the new object */
> > + struct symtab name_trans; /* filename transitions */
>
> What about using a bare hashtab to save the 4 bytes for the unused
> nprim member (+padding)?
That would mean losing the symtab_search()/symtab_insert() helpers
specialized for string keys. But it may be worth refactoring them down
into hashtab itself as an optimization.
> Also what about instead of storing an extra allocated u32 in the
> tables use the pointer itself as value?
That's possible, although quite hacky... But I guess if the casts are
hidden behind some helpers it could work... The same optimization
could be used for existing role_trans_datum.
<snip>
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
