netdev - Re: [PATCH] net: core: dev: Initialise napi state correctly

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: <CAOhhRxuga+S+JSt_v_K7uTu1xAxCV1-+y1VYCWXoya+gtZcWAg@mail.gmail.com>
Date:   Fri, 18 Jan 2019 18:52:36 +0000
From:   David McKay <mckay.david@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] net: core: dev: Initialise napi state correctly

On Fri, 18 Jan 2019 at 17:15, Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>
>
> On 01/18/2019 04:46 AM, Dave McKay wrote:
> > The state member of the napi_struct is not initialised correctly, it
> > sets the SCHED bit without initialising the state to zero first. This
> > results in peculiar behaviour if the original napi_struct didn't come
> > from a zero initialised region to start with.
> >
> > This patch just sets it directly using the appropriate bitfield
> > constant.
> >
> > Signed-off-by: Dave McKay <mckay.david@...il.com>
> > ---
> >  net/core/dev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 82f20022259d..250f97bf1973 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -6276,7 +6276,7 @@ void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
> >  #ifdef CONFIG_NETPOLL
> >       napi->poll_owner = -1;
> >  #endif
> > -     set_bit(NAPI_STATE_SCHED, &napi->state);
> > +     napi->state = NAPIF_STATE_SCHED;
> >       napi_hash_add(napi);
> >  }
> >  EXPORT_SYMBOL(netif_napi_add);
> >
>
> I am curious, which driver has exhibit any issue with current code ?
>

Hi Eric,
    As far as I know, none! I suspect all the napi_adds() are called on areas
that come from kzalloc(). I sampled a few and all of them would
not have hit this problem as the memory was allocated  in zero initialised
memory anyway.

I was working on an experimental out of tree driver that allocates the
napi struct through kzalloc().
I could not understand why kzalloc() worked but kmalloc() sometimes
didn't. I tracked it down to this.

Am I wrong here? Seemed to fix it for me!

Cheers!