netdev - Re: [PATCH v2 net] net/sched: act_ife: initalize ife->metalist earlier

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] [day] [month] [year] [list]

Message-ID: <CANn89iKjQR+xTGPbgSMjtxOQrKXUOFbwKqs_i95rfxtAgQAdkQ@mail.gmail.com>
Date:   Wed, 15 Jan 2020 08:16:32 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH v2 net] net/sched: act_ife: initalize ife->metalist earlier

On Wed, Jan 15, 2020 at 8:07 AM Davide Caratti <dcaratti@...hat.com> wrote:
>
>   On Wed, 2020-01-15 at 07:58 -0800, Eric Dumazet wrote:
> > It seems better to init ife->metalist earlier in tcf_ife_init()
> > to avoid the following crash :
>
> hi Eric, thanks for following up!
>
> [...]
> > ---
> > v2: addressed Davide feedback.
> >  net/sched/act_ife.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> > index 5e6379028fc392031f4b84599f666a2c61f071d2..ab748701374f65028c79cb789d065305430ea4c5 100644
> > --- a/net/sched/act_ife.c
> > +++ b/net/sched/act_ife.c
> > @@ -537,6 +537,9 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
> >       }
> >
> >       ife = to_ife(*a);
> > +     if (ret = ACT_P_CREATED)
> > +             INIT_LIST_HEAD(&ife->metalist);
> > +
>
> I didn't test the hunk above, but I think there is a typo, this assigns
> 'ret' rather than checking if it's equal to ACT_P_CREATED.
>
> It should be something like:
>
>         if (ret == ACT_P_CREATED)
>                 INIT_LIST_HEAD(&ife->metalist);
>
> correct?

Oh well, I should probably not attempt doing code changes in a bus ;)
Thanks, v3 will fix this.