netdev - struct stack initialization and padding bits

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

Date:   Fri, 27 Apr 2018 12:24:08 -0400
From:   Alexander Aring <aring@...atatu.com>
To:     netdev@...r.kernel.org
Cc:     stefan@....samsung.com, eric.dumazet@...il.com, jhs@...atatu.com,
        akpm@...ux-foundation.org, kernel@...atatu.com
Subject: struct stack initialization and padding bits

Hi,

I currently struggle with an issue with latest changes in inet_frag api.
I struggle at two points [0] which do a struct stack initialization and
[1] who do a memcmp() on this structure.

The struct "frag_lowpan_compare_key" has padding bits which are in my
case of system and compiler random initialized.

I wrote a small userspace testcase to confirm that (see below of this
mail). It compares with memcmp() a struct init by memset and per field
assign to a struct init like [0]. Of course I have 3 bytes hole after the
foo field.

My output is:

...
23:85:00:00:42:00:00:00
23:85:00:00:42:00:00:00
23:85:00:00:42:00:00:00
bingos 1000000
23:00:00:00:42:00:00:00

So all of my 1000000 tries are different between by field compare and memcmp().

My question is how kernel code handles this now when there is padding bytes?
According my simple test (if it's correct) we cannot use the style according
to [0].

I searched and found [3] which said the style of [0] should make
padding bits to zero. Also it was pointed out [4] that the compiler may
change this behavior.

- Alex

#include <inttypes.h>
#include <stdint.h>
#include <string.h>
#include <stdio.h>

struct foo {
        uint8_t foo;
        uint32_t bar;
};

static int decdump(const void *src, size_t size)
{
        const unsigned char *buf = src;
        int i;

        for (i = 0; i < size; i++) {
                if (i != size - 1)
                        printf("%02d:", buf[i]);
                else
                        printf("%02d", buf[i]);
        }

        printf("\n");
}

int main(int argc, const char *argv[])
{
        struct foo test2;
        uint32_t bingo = 0;
        int i, rc;

        memset(&test2, 0, sizeof(test2));
        test2.foo = 23;
        test2.bar = 42;

        for (i = 0; i < 1000000; i++) {
                struct foo test = {
                        .foo = 23,
                        .bar = 42,
                };

                rc = !!memcmp(&test, &test2, sizeof(test));

                if ((test2.foo == test.foo) &&
                    (test2.bar == test.bar)) {
                        if (rc) {
                                bingo++;
                                decdump(&test, sizeof(test));
                        }
                }
        }

        printf("bingos %" PRIu32 "\n", bingo);
        decdump(&test2, sizeof(test2));

        return 0;
}

[0] https://elixir.bootlin.com/linux/v4.17-rc2/source/net/ieee802154/6lowpan/reassembly.c#L78
[1] https://elixir.bootlin.com/linux/v4.17-rc2/source/net/ieee802154/6lowpan/reassembly.c#L594
[3] https://lwn.net/Articles/417994/
[4] https://lwn.net/Articles/417996/