2:46 p.m.
On Wednesday 21 December 2005 19:08, Amy Griffis wrote:
Thanks Amy for sending this. We at least have the beginnings of the new file
system audit patch to chew on. :) I only have a couple issues listed below.
They basically revolve around length checking of the netlink packet.
case AUDIT_ADD:
case AUDIT_DEL:
- if (nlh->nlmsg_len < sizeof(struct audit_rule))
+ if (nlmsg_len(nlh) < sizeof(struct audit_rule))
This checks the lower size bound, is there a check for the upper bound? Or
that the strings after buf[0] are in fact correctly sized? Does all the
lengths + struct add up to be equal to the payload length?
return -EINVAL;
/* fallthrough */
case AUDIT_LIST:
err = audit_receive_filter(nlh->nlmsg_type,
NETLINK_CB(skb).pid, - uid, seq,
data, loginuid); + uid, seq, data,
nlmsg_len(nlh), + loginuid);
+ break;
+ case AUDIT_ADD_RULE:
+ case AUDIT_DEL_RULE:
+ if (nlmsg_len(nlh) < sizeof(struct audit_rule_xprt))
Same.
+ return -EINVAL;
+ /* fallthrough */
+ case AUDIT_LIST_RULES:
+ err = audit_receive_filter(nlh->nlmsg_type,
NETLINK_CB(skb).pid, + uid, seq,
data, nlmsg_len(nlh), + loginuid);
break;
case AUDIT_SIGNAL_INFO:
sig_data.uid = audit_sig_uid;
right); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index a3a3275..1fd4b2a 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -40,52 +40,253 @@ struct list_head audit_filter_list[AUDIT
#endif
};
-/* Copy rule from user-space to kernel-space. Called from
- * audit_add_rule during AUDIT_ADD. */
-static inline int audit_copy_rule(struct audit_rule *d, struct audit_rule
*s) +/* Unpack a filter field's string representation from user-space
+ * buffer. */
+static inline char *audit_unpack_string(void **bufp, int *remaining, int
len) {
I couldn't find callers for this. Is there any chance that len or remaining
could be negative? If not, lets make them unsigned.
+ char *str;
+
+ if (!*bufp || (len > *remaining))
+ return ERR_PTR(-EINVAL);
len should always be <= PATH_MAX*2. That must be checked for as well or there
could be wrapping.
+/* Common user-space to kernel rule translation. */
+static struct audit_entry *audit_to_entry_common(struct audit_rule *rule)
+{
+ unsigned listnr;
+ struct audit_entry *entry;
+ int i, err;
+
+ err = -EINVAL;
+ listnr = rule->flags & ~AUDIT_FILTER_PREPEND;
+ switch(listnr) {
+ default:
+ goto exit_err;
+ case AUDIT_FILTER_USER:
+ case AUDIT_FILTER_TYPE:
+#ifdef CONFIG_AUDITSYSCALL
+ case AUDIT_FILTER_ENTRY:
+ case AUDIT_FILTER_EXIT:
+ case AUDIT_FILTER_TASK:
+#endif
+ ;
+ }
+ if (rule->action != AUDIT_NEVER && rule->action != AUDIT_POSSIBLE
&& + rule->action != AUDIT_ALWAYS)
+ goto exit_err;
+ if (rule->field_count < 0 || rule->field_count > AUDIT_MAX_FIELDS)
+ goto exit_err;
field_count is u32 & cannot be less than 0. It should be checked to make sure
its not 0, though.
+
+ err = -ENOMEM;
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ goto exit_err;
+ memset(&entry->rule, 0, sizeof(struct audit_krule));
+
+ entry->rule.flags = rule->flags & AUDIT_FILTER_PREPEND;
+ entry->rule.listnr = listnr;
+ entry->rule.action = rule->action;
+ entry->rule.field_count = rule->field_count;
+
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ entry->rule.mask[i] = rule->mask[i];
+
+ return entry;
+
+exit_err:
+ return ERR_PTR(err);
+}
+
+/* Translate struct audit_rule to kernel's rule respresentation.
+ * Exists for backward compatibility with userspace. */
+static struct audit_entry *audit_rule_to_entry(struct audit_rule *rule)
+{
+ struct audit_entry *entry;
+ int err = 0;
int i;
- if (s->action != AUDIT_NEVER
- && s->action != AUDIT_POSSIBLE
- && s->action != AUDIT_ALWAYS)
- return -1;
- if (s->field_count < 0 || s->field_count > AUDIT_MAX_FIELDS)
- return -1;
Same as above. sb == 0 rather than < 0.
- if ((s->flags & ~AUDIT_FILTER_PREPEND) >=
AUDIT_NR_FILTERS)
- return -1;
-
- d->flags = s->flags;
- d->action = s->action;
- d->field_count = s->field_count;
- for (i = 0; i < d->field_count; i++) {
- d->fields[i] = s->fields[i];
- d->values[i] = s->values[i];
+ entry = audit_to_entry_common(rule);
+ if (IS_ERR(entry))
+ goto exit_nofree;
+
+ for (i = 0; i < rule->field_count; i++) {
+ struct audit_field *f = &entry->rule.fields[i];
+
+ if (rule->fields[i] & AUDIT_UNUSED_BITS) {
+ err = -EINVAL;
+ goto exit_free;
+ }
+
+ f->op = rule->fields[i] & (AUDIT_NEGATE|AUDIT_OPERATORS);
+ f->type = rule->fields[i] &
(~AUDIT_NEGATE|AUDIT_OPERATORS); + f->val = rule->values[i];
+
+ entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;
+ if (f->op & AUDIT_NEGATE)
+ f->op |= AUDIT_NOT_EQUAL;
+ else if (!(f->op & AUDIT_OPERATORS))
+ f->op |= AUDIT_EQUAL;
+ f->op &= ~AUDIT_NEGATE;
}
- for (i = 0; i < AUDIT_BITMASK_SIZE; i++) d->mask[i] = s->mask[i];
- return 0;
+
+exit_nofree:
+ return entry;
+
+exit_free:
+ kfree(entry);
+ return ERR_PTR(err);
}
-/* Check to see if two rules are identical. It is called from
- * audit_add_rule during AUDIT_ADD and
- * audit_del_rule during AUDIT_DEL. */
-static int audit_compare_rule(struct audit_rule *a, struct audit_rule *b)
+/* Translate struct audit_rule_xprt to kernel's rule respresentation. */
+static struct audit_entry *audit_xprt_to_entry(struct audit_rule_xprt
*xprt, + int dlen)
{
+ int err = 0;
+ struct audit_entry *entry;
+ void *bufp;
+ /* int remaining = dlen - sizeof(struct audit_rule_xprt); */
int i;
- if (a->flags != b->flags)
- return 1;
+ entry = audit_to_entry_common((struct audit_rule *)xprt);
+ if (IS_ERR(entry))
+ goto exit_nofree;
+
+ bufp = xprt->buf;
What uses bufp?
+/* Pack a filter field's string representation into data block.
*/
+static inline int audit_pack_string(void **bufp, char *str)
What calls this?
+{
+ int len = strlen(str);
Why use int? size_t is natural size.
+ memcpy(*bufp, str, len);
+ *bufp += len;
+
+ return len;
+}
+
-Steve