On Mon, May 14, 2018 at 05:27:07PM +0300, Vlad Buslov wrote:
...
> @@ -1052,6 +1088,36 @@ static int tca_action_flush(struct net *net, struct
> nlattr *nla,
> return err;
> }
>
> +static int tcf_action_delete(struct net *net, struct list_head *actions,
> + struct netlink_ext_ack *extack)
> +{
> + int ret;
Reverse christmass tree.. this line should be the last in variable
declarations.
> + struct tc_action *a, *tmp;
> + char kind[IFNAMSIZ];
> + u32 act_index;
> +
> + list_for_each_entry_safe(a, tmp, actions, list) {
> + const struct tc_action_ops *ops = a->ops;
> +
> + /* Actions can be deleted concurrently
> + * so we must save their type and id to search again
> + * after reference is released.
> + */
> + strncpy(kind, a->ops->kind, sizeof(kind) - 1);
This may be problematic. Why strncpy here?
a->ops->kind is also of size IFNAMSIZ. If a->ops->kind is actually
IFNAMSIZ-1 long, kind here won't be NULL terminated, as kind is not
initialized and strncpy won't add the NULL.
> + act_index = a->tcfa_index;
> +
> + list_del(&a->list);
> + if (tcf_action_put(a))
> + module_put(ops->owner);
> +
> + /* now do the delete */
> + ret = tcf_action_del_1(net, kind, act_index, extack);
> + if (ret < 0)
> + return ret;
> + }
> + return 0;
> +}
