netdev - Re: [PATCH ethtool 2/3] Add IPv6 support to NFC

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]

Date:	Wed, 16 Mar 2016 17:15:41 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Edward Cree <ecree@...arflare.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH ethtool 2/3] Add IPv6 support to NFC

On Wed, 2016-03-16 at 14:53 +0000, Edward Cree wrote:
> On 13/03/16 16:43, Ben Hutchings wrote:
> > 
> > On Mon, 2016-02-15 at 14:59 +0000, Edward Cree wrote:
> > > 
> > > Signed-off-by: Edward Cree <ecree@...arflare.com>
> > [...]
> > > 
> > > @@ -950,6 +1154,19 @@ static int rxclass_get_mask(char *str, unsigned char *p,
> > >                 *(__be32 *)&p[opt->moffset] = ~val;
> > >                 break;
> > >         }
> > > +       case OPT_IP6: {
> > > +               __be32 val[4];
> > > +               int i;
> > > +               err = rxclass_get_ipv6(str, val);
> > > +               if (err)
> > > +                       return -1;
> > > +               for (i = 0; i < 4; i++) {
> > > +                       ((__be32 *)&p[opt->offset])[i] = val[i];
> > > +                       if (opt->moffset >= 0)
> > > +                               ((__be32 *)&p[opt->moffset])[i] = ~val[i];
> > This pointer arithmetic looks terrible.  I think memcpy() would be much
> > clearer here.
> I've changed the version in rxclass_get_val to use memcpy() (and memset() the
> mask).  Unfortunately, we can't do that here, because we need to complement
> the mask valueas we go, and afaik there's no library function to copy-and-
> complement a byte array.

Sorry, I missed the inversion.  But it would still be cleaner to use a
local variable:

			__be32 *field = (__be32 *)&p[opt->offset];

> Glibc does, however, have a function memfrob(), which XORs every byte of an
> arraywiththe constant 42.  Useful feature, that.

It is!

#include <string.h>

int main(void)
{
    char answer;
    memset(&answer, 0, 1);
    memfrob(&answer, 1);
    return answer;
}

> On the other hand, the quoted code is still wrong because it's also writing
> throughopt->offset and checking for opt->moffset>= 0, both daft copy-and-
> paste errors onmypart.  Will fix in next version.
> > 
> > I won't apply patches labelled as "confidential".  You need to stop
> > including this nonsense in your public messages (I thought you fixed
> > this once before).
> In theory it's been fixed harder now - please let me know if not.

Looks like you're sending two copies, one with and one without.  Which
works for me, though it might annoy some recipients...

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)