One additional remark: When applying your patch on Dave Millers net-next tree there are some offsets in the patch ... Please send the next patch based on the net-next tree, as this would be the tree, it will be applied to. See URLs at: http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git Tnx! Oliver On 29.06.2012 17:44, Oliver Hartkopp wrote: > Hello Rostislav, > > looks really good now. > > 1. Your Signed-off-by: is missing. > > 2. One remark to a removed length check: > > (..) > >> +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; > > ??? > >> + 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? > >> +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 > -- > To unsubscribe from this list: send the line "unsubscribe linux-can" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe lartc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
