linux-kernel - Re: [PATCH] netfilter: ip_vs_sync: fix bogus maybe-uninitialized warning

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]

Date:   Mon, 24 Oct 2016 22:21:59 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Julian Anastasov <ja@....bg>
Cc:     Wensong Zhang <wensong@...ux-vs.org>,
        Simon Horman <horms@...ge.net.au>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Patrick McHardy <kaber@...sh.net>,
        Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
        "David S. Miller" <davem@...emloft.net>,
        Quentin Armitage <quentin@...itage.org.uk>,
        netdev@...r.kernel.org, lvs-devel@...r.kernel.org,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] netfilter: ip_vs_sync: fix bogus maybe-uninitialized warning

On Monday, October 24, 2016 10:47:54 PM CEST Julian Anastasov wrote:
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index 1b07578bedf3..9350530c16c1 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -283,6 +283,7 @@ struct ip_vs_sync_buff {
> >   */
> >  static void ntoh_seq(struct ip_vs_seq *no, struct ip_vs_seq *ho)
> >  {
> > +     memset(ho, 0, sizeof(*ho));
> >       ho->init_seq       = get_unaligned_be32(&no->init_seq);
> >       ho->delta          = get_unaligned_be32(&no->delta);
> >       ho->previous_delta = get_unaligned_be32(&no->previous_delta);
> 
>         So, now there is a double write here?

Correct. I would hope that a sane version of gcc would just not
perform the first write. What happens instead is that the version
that produces the warning here moves the initialization to the
top of the calling function.

>         What about such constructs?:
> 
>         *ho = (struct ip_vs_seq) {
>                 .init_seq       = get_unaligned_be32(&no->init_seq),
>                 ...
>         };
> 
>         Any difference in the compiled code or warnings?

Yes, it's one of many things I tried. What happens here is that
the warning remains as long as all fields are initialized together,
e.g. these two produces the same warning:

a)
    ho->init_seq       = get_unaligned_be32(&no->init_seq);
    ho->delta          = get_unaligned_be32(&no->delta);
    ho->previous_delta = get_unaligned_be32(&no->previous_delta);

b)
   *ho = (struct ip_vs_seq) {
       .init_seq       = get_unaligned_be32(&no->init_seq);
       .delta          = get_unaligned_be32(&no->delta);
       .previous_delta = get_unaligned_be32(&no->previous_delta);
   };

but this one does not:

c)
   *ho = (struct ip_vs_seq) {
       .delta          = get_unaligned_be32(&no->delta);
       .previous_delta = get_unaligned_be32(&no->previous_delta);
   };
   ho->init_seq       = get_unaligned_be32(&no->init_seq);

I have absolutely no idea what is going on inside of gcc here.

	Arnd