On 15/10/18, Scott Matheina wrote: > On 10/14/2015 04:54 PM, Paul Moore wrote: > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote: > >> Signed-off-by: Scott Matheina <sc...@matheina.com> > >> --- > >> kernel/auditfilter.c | 17 ++++++++++------- > >> 1 file changed, 10 insertions(+), 7 deletions(-) > > Sorry for the delay in reviewing this, comments inline ... > > > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > >> index 7714d93..774f9ad 100644 > >> --- a/kernel/auditfilter.c > >> +++ b/kernel/auditfilter.c > >> @@ -39,13 +39,13 @@ > >> * Locking model: > >> * > >> * audit_filter_mutex: > >> - * Synchronizes writes and blocking reads of audit's > >> filterlist > >> - * data. Rcu is used to traverse the filterlist and access > >> - * contents of structs audit_entry, audit_watch and opaque > >> - * LSM rules during filtering. If modified, these > >> structures > >> - * must be copied and replace their counterparts in the > >> filterlist. > >> - * An audit_parent struct is not accessed during > >> filtering, so may > >> - * be written directly provided audit_filter_mutex is held. > >> + * Synchronizes writes and blocking reads of audit's > >> filterlist > >> + * data. Rcu is used to traverse the filterlist and access > >> + * contents of structs audit_entry, audit_watch and opaque > >> + * LSM rules during filtering. If modified, these > >> structures > >> + * must be copied and replace their counterparts in the > >> filterlist. > >> + * An audit_parent struct is not accessed during > >> filtering, so may > >> + * be written directly provided audit_filter_mutex is held. > >> */ > > Okay, that's fine. > > > >> /* Audit filter lists, defined in <linux/audit.h> */ > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head) > >> { > >> struct audit_entry *e = container_of(head, struct audit_entry, rcu); > >> audit_free_rule(e); > >> + > >> } > > Why? > > I was following the error messages in checkpatch.pl, but the warning > went away after adding this line. No problem with the code.
That sounds like a bug in checkpatch.pl, since that blank line should be tween the declaration and the function call. > >> /* Initialize an audit filterlist entry. */ > >> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES]; > >> int __init audit_register_class(int class, unsigned *list) > >> { > >> __u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL); > >> + > >> if (!p) > >> return -ENOMEM; > > Okay. > > > >> while (*list != ~0U) { > >> + > >> unsigned n = *list++; > >> if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) { > >> kfree(p); > > Why? > > This is the same as above. Just going through the checkpatch.pl > script, and looking for warnings to fix. Again, another manifestation of that bug? That blank line should be after the declaration and before the if statement. > As you might have guessed, this is one of my first patches. I wasn't > sure if a patch like this would even get reviewed, and responded to. > I'm subscribed to the linux-kernel mail group, and seeing what is > acceptable. > > Thanks for the review. I don't plan on making a habit of submitting > such incredibly trivial patches, but you have to start somewhere, and > I thought it'd be hard to screw up by fixing a couple of trivial style > errors. Well, I agree, you have to start somewhere... Too bad you hit a bug in checkpatch.pl! - RGB -- Richard Guy Briggs <rbri...@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit