LGTM
On Mon, Sep 9, 2013 at 12:05 PM, Stephen Smalley <[email protected]> wrote: > On 09/09/2013 02:43 PM, Stephen Smalley wrote: > > On 09/09/2013 02:22 PM, Stephen Smalley wrote: > >> On 09/09/2013 01:01 PM, Stephen Smalley wrote: > >>> On 09/09/2013 09:47 AM, Stephen Smalley wrote: > >>>> On 09/06/2013 03:50 PM, Joshua Brindle wrote: > >>>>> Add libaudit support for adding directory watch rules. > >>>>> > >>>>> Add rule parsing support to auditd. > >>>>> > >>>>> Rule format matches auditctl. Currently only supports -w and -e. > >>>>> > >>>>> Change-Id: I8bdaea1b5e2a216eec79cd8c9dae583de8295d26 > >>>>> > >>>>> Signed-off-by: Joshua Brindle <[email protected]> > >>>> > >>>> Maybe a bug in user, but I did this: > >>>> - applied patch and rebuilt, > >>>> - reflashed and booted, > >>>> - created a /data/misc/audit/audit.rules file that contained: > >>>> -w /data/system -p wa > >>>> - adb reboot > >>>> - adb logcat > logcat.txt > >>>> - adb shell su 0 cat /proc/kmsg > dmesg.txt > >>>> > >>>> logcat.txt showed: > >>>> --------- beginning of /dev/log/system > >>>> I/auditd ( 119): Starting up > >>>> I/audit_log( 119): Previous audit logfile detected, rotating > >>>> E/audit_rules( 119): -w /data/system -p wa > >>>> > >>>> And then nothing else from auditd. > >>>> > >>>> /data/misc/audit/audit.log has no entries other than the usual: > >>>> type=2000 msg=audit(0.710:1): initialized > >>>> type=1403 msg=audit(1378733645.695:2): policy loaded auid=4294967295 > >>>> ses=4294967295 > >>>> type=1404 msg=audit(1378733645.695:3): enforcing=1 old_enforcing=0 > >>>> auid=4294967295 ses=4294967295 > >>>> type=1403 msg=audit(1378733647.665:4): policy loaded auid=4294967295 > >>>> ses=4294967295 > >>>> type=1404 msg=audit(1378733830.500:5): enforcing=0 old_enforcing=1 > >>>> auid=4294967295 ses=4294967295 > >>>> > >>>> Creating and deleting files under /data/system appears to do nothing. > >>>> What did I miss? > >>> > >>> So I re-tested with our kernel (i.e. > >>> > TARGET_PREBUILT_KERNEL=/path/to/seandroid/kernel/exynos/arch/arm/boot/zImage) > >>> and that did generate the expected audit records. I'm guessing that is > >>> because we have a patch in our kernel tree that enables audit by > >>> default. Since your patch implements the -e (enable) support, I > thought > >>> I would try that on an unmodified kernel by putting > >>> -e 1 > >>> -w /data/system -p wa > >>> into audit.rules. > >>> > >>> But we then get a parse error from audit_rules, > >>> E/audit_rules( 2504): Could not read audit rules > >>> E/auditd ( 2504): error reading audit rules: Try again > >>> > >>> Am I doing something wrong or is the parser broken? > >> > >> Actually, it appears that audit_set_enabled() is failing, but your error > >> handling code doesn't report it there so it ends up being reported as a > >> failure reading the audit rules. > >> > >> And that in turn appears to be a problem in audit_get_reply(), not > >> something you changed. Bill, your code in libaudit.c:audit_get_reply() > >> is bailing with an error in the EAGAIN case rather than retrying despite > >> the comment to the contrary. > > > > Also, I think upstream auditd always does an audit_set_enabled(fd, 1) > > during startup unless told to do otherwise. > > So, with this patch applied on top of yours, it seems to work correctly > on the prebuilt kernel. > > > > -- Respectfully, William C Roberts
