linux-kernel - Re: [tip:x86/core 1/1] arch/x86/um/../lib/csum-partial_64.c:98:12: error: implicit declaration of function 'load_unaligned

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: <CAFUsyf+5zp+p_0TPFLr-fMNry0M_CnNAFDG30PKDuy2jA5MhNw@mail.gmail.com>
Date:   Wed, 24 Nov 2021 21:41:19 -0600
From:   Noah Goldstein <goldstein.w.n@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Johannes Berg <johannes@...solutions.net>, alexanderduyck@...com,
        kbuild-all@...ts.01.org, open list <linux-kernel@...r.kernel.org>,
        linux-um@...ts.infradead.org, lkp@...el.com, peterz@...radead.org,
        X86 ML <x86@...nel.org>
Subject: Re: [tip:x86/core 1/1] arch/x86/um/../lib/csum-partial_64.c:98:12:
 error: implicit declaration of function 'load_unaligned_zeropad'

On Wed, Nov 24, 2021 at 8:56 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Wed, Nov 24, 2021 at 5:59 PM Noah Goldstein <goldstein.w.n@...il.com> wrote:
> >
>
> >
> > Hi, I'm not sure if this is intentional or not, but I noticed that the output
> > of 'csum_partial' is different after this patch. I figured that the checksum
> > algorithm is fixed so just wanted mention it incase its a bug. If not sorry
> > for the spam.
> >
> > Example on x86_64:
> >
> > Buff: [ 87, b3, 92, b7, 8b, 53, 96, db, cd, 0f, 7e, 7e ]
> > len : 11
> > sum : 0
> >
> > csum_partial new : 2480936615
> > csum_partial HEAD: 2472089390
>
> No worries.
>
> skb->csum is 32bit, but really what matters is the 16bit folded value.
>
> So make sure to apply csum_fold() before comparing the results.
>
> A minimal C and generic version of csum_fold() would be something like
>
> static unsigned short csum_fold(u32 csum)
> {
>   u32 sum = csum;
>   sum = (sum & 0xffff) + (sum >> 16);
>   sum = (sum & 0xffff) + (sum >> 16);
>   return ~sum;
> }
>
> I bet that csum_fold(2480936615) == csum_fold(2472089390)
>

Correct :)

The outputs seem to match if `buff` is aligned to 64-bit. Still see
difference with `csum_fold(csum_partial())` if `buff` is not 64-bit aligned.

The comment at the top says it's "best" to have `buff` 64-bit aligned but
the code logic seems meant to support the misaligned case so not
sure if it's an issue.

Example:

csum_fold(csum_partial) new : 0x3764
csum_fold(csum_partial) HEAD: 0x3a61

buff        : [ 11, ea, 75, 76, e9, ab, 86, 48 ]
buff addr   : ffff88eaf5fb0001
len         : 8
sum_in      : 25

> It would be nice if we had a csum test suite, hint, hint ;)

Where in the kernel would that belong?

>
> Thanks !