Re: [PATCH net-next] em_canid: Ematch rule to match CAN frames according to their identifiers
flat view
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: 2012-06-29 15:44:10
Also in:
linux-can
Hello Rostislav, looks really good now. 1. Your Signed-off-by: is missing. 2. One remark to a removed length check: (..)
quoted hunk ↗ jump to hunk
+static int em_canid_change(struct tcf_proto *tp, void *data, int len, + struct tcf_ematch *m) +{ + struct can_filter *conf = data; /* Array with rules, + * fixed size EM_CAN_RULES_SIZE + */ + struct canid_match *cm; + struct canid_match *cm_old = (struct canid_match *) m->data; + int i; + int rulescnt; +
What about a zero length check here? if (!len) return -EINVAL; ???
quoted hunk ↗ jump to hunk
+ if (len % sizeof(struct can_filter)) + return -EINVAL; + + if (len > sizeof(struct can_filter) * EM_CAN_RULES_MAX) + return -EINVAL; + + rulescnt = len / sizeof(struct can_filter); + + cm = kzalloc(sizeof(struct canid_match) + sizeof(struct can_filter) * + rulescnt, GFP_KERNEL); + if (!cm) + return -ENOMEM;
The length could alternatively be checked here too http://lxr.linux.no/#linux+v3.4.4/net/sched/ematch.c#L235 if em->ops->datalen is set. But here's no .datalen = sizeof(struct can_filter), defined, right?
quoted hunk ↗ jump to hunk
+static struct tcf_ematch_ops em_canid_ops = { + .kind = TCF_EM_CANID, + .change = em_canid_change, + .match = em_canid_match, + .destroy = em_canid_destroy, + .dump = em_canid_dump, + .owner = THIS_MODULE, + .link = LIST_HEAD_INIT(em_canid_ops.link) +};
Regards, Oliver