netdev - Re: [Patch net-next] net_sched: properly handle failure case of tcf_exts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <78193bfb-33c4-4dc7-fdd9-18bb3d1084ce@mojatatu.com>
Date:   Mon, 22 Aug 2016 08:06:03 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org
Subject: Re: [Patch net-next] net_sched: properly handle failure case of
 tcf_exts_init()


Just small comment below:

On 16-08-19 03:36 PM, Cong Wang wrote:
> diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
> index 944c8ff..d950070 100644
> --- a/net/sched/cls_tcindex.c
> +++ b/net/sched/cls_tcindex.c
> @@ -219,10 +219,10 @@ static const struct nla_policy tcindex_policy[TCA_TCINDEX_MAX + 1] = {
>  	[TCA_TCINDEX_CLASSID]		= { .type = NLA_U32 },
>  };
>
> -static void tcindex_filter_result_init(struct tcindex_filter_result *r)
> +static int tcindex_filter_result_init(struct tcindex_filter_result *r)
>  {
>  	memset(r, 0, sizeof(*r));
> -	tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
> +	return tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
>  }
>
>  static void __tcindex_partial_destroy(struct rcu_head *head)
> @@ -233,23 +233,57 @@ static void __tcindex_partial_destroy(struct rcu_head *head)
>  	kfree(p);
>  }
>
> +static void tcindex_free_perfect_hash(struct tcindex_data *cp)
> +{
> +	int i;
> +
> +	for (i = 0; i < cp->hash; i++)
> +		tcf_exts_destroy(&cp->perfect[i].exts);
> +	kfree(cp->perfect);
> +}
> +
> +static int tcindex_alloc_perfect_hash(struct tcindex_data *cp)
> +{
> +	int i, err = 0;
> +
> +	cp->perfect = kcalloc(cp->hash, sizeof(struct tcindex_filter_result),
> +			      GFP_KERNEL);
> +	if (!cp->perfect)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < cp->hash; i++) {
> +		err = tcf_exts_init(&cp->perfect[i].exts,
> +				    TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
> +		if (err < 0)
> +			goto errout;
> +	}
> +
> +	return 0;
> +
> +errout:
> +	tcindex_free_perfect_hash(cp);
> +	return err;
> +}
> +


If you fail tcindex_alloc_perfect_hash somewhere freeing actions in
tcindex_free_perfect_hash() via tcf_exts_destroy require checking
if exts->actions require a check?

Otherwise, looks good.

Acked-by: Jamal Hadi Salim <jhs@...atatu.com>

cheers,
jamal