On Wed, Aug 30, 2017 at 2:49 AM, Nikolay Aleksandrov
<niko...@cumulusnetworks.com> wrote:
> It is very unlikely to happen but the backlogs memory allocation
> could fail and will free q->flows, but then ->destroy() will free
> q->flows too. For correctness remove the first free and let ->destroy
> clean up.
>
> Fixes: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> Signed-off-by: Nikolay Aleksandrov <niko...@cumulusnetworks.com>
> ---
> net/sched/sch_fq_codel.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
> index 337f2d6d81e4..2c0c05f2cc34 100644
> --- a/net/sched/sch_fq_codel.c
> +++ b/net/sched/sch_fq_codel.c
> @@ -491,10 +491,8 @@ static int fq_codel_init(struct Qdisc *sch, struct
> nlattr *opt)
> if (!q->flows)
> return -ENOMEM;
> q->backlogs = kvzalloc(q->flows_cnt * sizeof(u32),
> GFP_KERNEL);
> - if (!q->backlogs) {
> - kvfree(q->flows);
> + if (!q->backlogs)
> return -ENOMEM;
> - }
This is fine. Or we can NULL it after kvfree().
I have no preference here. The only difference here is if we still
expect ->init() to cleanup its own failure.
