netdev - Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's

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: <4B0407C7.1030503@gmail.com>
Date:	Wed, 18 Nov 2009 09:42:15 -0500
From:	William Allen Simpson <william.allen.simpson@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Joe Perches <joe@...ches.com>,
	David Miller <davem@...emloft.net>
Subject: Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie
 option, extend existing struct's

Eric Dumazet wrote:
> William Allen Simpson a écrit :
>> There is nothing yet in this patch series to send data with a SYN.  Back in
>> early October, David required that the various s_data and cookie structures
>> be compressed and consolidated.  So, for the client side, the cookie_*
>> fields are filled and the s_data_* fields are zero (ignored), while the
>> server side can have both filled.
>>
>> Moreover, *this* patch does nothing other than allocate and deallocate the
>> structure, zero filled by kzalloc().
>>
>> SYN data will be implemented (much) later.
> 
> okay
> 
Still no technical corrections.  Seeking Acks.

Trying to get review back on track.  The patch is still the same.

This patch allocates and deallocates the structures.  One main
difference (off the top of my head) from Alan's original patch is
that it uses sk->sk_allocation instead of GFP_ATOMIC in a couple of
places.  David Miller wanted to reduce the number of atomic
allocations, and sk->sk_allocation can be GFP_KERNEL or GFP_ATOMIC.

AFAICT, it's usually GFP_KERNEL in both tcp_v4_init_sock() and
tcp_v6_init_sock(), as they're called by inet_create() via
sk->sk_prot->init() shortly after calling sock_init_data() that set
sk->sk_allocation to GFP_KERNEL.

I used the existing variable just in case there are other code paths
that I've missed, and just in case there are future changes adding
code paths.

Limited multiple (=) assignments to only 2 per line for readability
(that in existing code have 4 per line and run well beyond 80
characters, triggering a warning in scripts/checkpatch.pl).

Data structures are carefully composed to require minimal additions.
For example, the struct tcp_options_received cookie_plus variable fits
between existing 16-bit and 8-bit variables, requiring no additional
space (taking alignment into consideration).  There are no additions to
tcp_request_sock, and only 1 pointer in tcp_sock.

View attachment "TCPCT+1d6++.patch" of type "text/plain" (10949 bytes)