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 <scott(a)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.
> /* 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.
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.