[systemd-devel] [PATCH] ima-setup: write policy one line at a time

[Zbigniew Jędrzejewski-Szmek]

On Thu, Jun 11, 2015 at 11:28:06AM +0200, Lennart Poettering wrote:
> On Thu, 11.06.15 00:34, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> 
> > On Thu, Jun 11, 2015 at 01:16:47AM +0200, Lennart Poettering wrote:
> > > On Wed, 10.06.15 15:38, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> > > 
> > > > ima_write_policy() expects data to be written as one or more
> > > > rules, no more than PAGE_SIZE at a time. Easiest way to ensure
> > > > that we are not splitting rules is to read and write on line at
> > > > a time.
> > > > 
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1226948
> > > > ---
> > > >  src/core/ima-setup.c | 39 +++++++++++++++++----------------------
> > > >  1 file changed, 17 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
> > > > index 4d8b638115..5b3d16cd31 100644
> > > > --- a/src/core/ima-setup.c
> > > > +++ b/src/core/ima-setup.c
> > > > @@ -23,9 +23,6 @@
> > > >  
> > > >  #include <unistd.h>
> > > >  #include <errno.h>
> > > > -#include <fcntl.h>
> > > > -#include <sys/stat.h>
> > > > -#include <sys/mman.h>
> > > >  
> > > >  #include "ima-setup.h"
> > > >  #include "util.h"
> > > > @@ -36,20 +33,19 @@
> > > >  #define IMA_POLICY_PATH "/etc/ima/ima-policy"
> > > >  
> > > >  int ima_setup(void) {
> > > > -        int r = 0;
> > > > -
> > > >  #ifdef HAVE_IMA
> > > > -        _cleanup_close_ int policyfd = -1, imafd = -1;
> > > > -        struct stat st;
> > > > -        char *policy;
> > > > +        _cleanup_fclose_ FILE *input = NULL;
> > > > +        _cleanup_close_ int imafd = -1;
> > > > +        char line[LINE_MAX];
> > > 
> > > Hmm, I wonder if this might bite us. LINE_MAX is a good choice as max
> > > line length for formats we define in systemd, but the question of
> > > course is what the the max line length is for IMA...
> >
> > It's PAGE_SIZE ;) Making this dynamic doesn't make much sense to me,
> > but we could make it 4096, as this is the lowest (and common) size.
> 
> I don't think this is actually really that bad:
> 
>         _cleanup_free_ void *line = NULL;
>         line = malloc(page_size());
> 
> Or, we could even just do alloca(page_size())...
Either would break FOR_EACH_LINE, but line[page_size()] should work.

https://github.com/systemd/systemd/pull/167

What I don't like about having a non-fixed value for the line size is
that the syntactic validity of configuration depends on the kernel you
are running. In practice not an issue, unless you like really long
lines.

@Mimi: could you check that the patch works for you?

Zbyszek