William Allen Simpson a écrit :

The (buggy?) syncookie code that I'm avoiding/replacing is:

static DEFINE_PER_CPU(__u32 [16 + 5 + SHA_WORKSPACE_WORDS],
              ipv4_cookie_scratch);

static u32 cookie_hash(__be32 saddr, __be32 daddr, __be16 sport, __be16
dport,
               u32 count, int c)
{
    __u32 *tmp = __get_cpu_var(ipv4_cookie_scratch);

    memcpy(tmp + 4, syncookie_secret[c], sizeof(syncookie_secret[c]));
    tmp[0] = (__force u32)saddr;
    tmp[1] = (__force u32)daddr;
    tmp[2] = ((__force u32)sport << 16) + (__force u32)dport;
    tmp[3] = count;
    sha_transform(tmp + 16, (__u8 *)tmp, tmp + 16 + 5);

    return tmp[17];
}

It appears to me that the operations could be interrupted at any time, and
the fairly expensive sha transform (or probably any of the assignments)
could be corrupted?

That is, cpu0 grabs a scratch area, copies some data into it, interrupts,
cpu0 comes along again with another packet, points into the same workspace,
mashes the data, while cpu1 completes the previous operation with the
old tmp pointer on the stack.

Worse, this is called twice, each time getting tmp, and mashing the data,
and at the same time others are calling it twice more for verification.

Since syncookies only occur under stress, the code isn't well tested, and
the only symptom would be the returned packet would be dropped after
failing to verify.  Since this only happens when lots of packets are
arriving, dropped packets probably aren't noticed.

However, that would be unacceptable for my code.

cookie_hash() runs in a non preemptable context. CPU cannot change under us.

(or else, we would not use __get_cpu_var(ipv4_cookie_scratch); )

And of course, each cpu gets its own scratch area, thanks to __get_cpu_var()