On Monday, November 21, 2016 4:50:03 PM EST Paul Moore wrote:
> On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb@xxxxxxxxxx> wrote:
> > The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same
> > thing as the event type and is missing the uid and results field. The
> > bigger issue is that in some failure cases no event is emitted. This patch
> > fixes the noted problems.
> >
> > Signed-off-by: Steve Grubb <sgrubb@xxxxxxxxxx>
>
> First off, for patches such as these, I think it is good to CC the
> affected subsystem, SELinux in this case (fixed).
OK.
> Beyond that, I'm a little concerned that you adding fields to record
> in the middle. In the past you've warned against inserting fields in
> the middle of the record, or reordering fields in general ("you'll
> break the world") due to some poor userspace practices, yet you do
> these exact things when it suits you.
Its not when its suits me, its when it makes sense. This aligns
AUDIT_MAC_POLICY_LOAD with more event patterns. What I think the solution is
is to have a logging function that takes care of the sequence so that people
do not have to hand code it like this is.
> We need a consistent message when it comes to userspace record
> processing so we know what we can do in the kernel without causing
> massive failure.
I'd like to finish aligning all simple events to a standard for the next kernel
release.
-Steve
> > --- vanilla-4.9-rc5.orig/security/selinux/selinuxfs.c 2016-11-16
> > 15:16:34.738723900 -0500 +++
> > linux-4.9.0-0.rc5.git0.1.fc24.x86_64/security/selinux/selinuxfs.c
> > 2016-11-21 12:16:08.046787604 -0500 @@ -494,6 +494,7 @@ static ssize_t
> > sel_write_load(struct fil
> >
> > {
> >
> > ssize_t length;
> > void *data = NULL;
> >
> > + unsigned int result = 0;
> >
> > mutex_lock(&sel_mutex);
> >
> > @@ -525,24 +526,26 @@ static ssize_t sel_write_load(struct fil
> >
> > length = sel_make_bools();
> > if (length)
> >
> > - goto out1;
> > + goto out;
> >
> > length = sel_make_classes();
> > if (length)
> >
> > - goto out1;
> > + goto out;
> >
> > length = sel_make_policycap();
> > if (length)
> >
> > - goto out1;
> > + goto out;
> >
> > length = count;
> >
> > + result = 1;
> >
> > -out1:
> >
> > +out:
> > audit_log(current->audit_context, GFP_KERNEL,
> > AUDIT_MAC_POLICY_LOAD,
> >
> > - "policy loaded auid=%u ses=%u",
> > + "uid=%u auid=%u ses=%u res=%u",
> > + from_kuid(&init_user_ns, task_uid(current)),
> >
> > from_kuid(&init_user_ns, audit_get_loginuid(current)),
> >
> > - audit_get_sessionid(current));
> > -out:
> > + audit_get_sessionid(current), result);
> > +
> >
> > mutex_unlock(&sel_mutex);
> > vfree(data);
> > return length;
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/linux-audit
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
