netdev - Re: [net-next-2.6 PATCH v6 3/7 RFC] TCPCT part 1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE

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: <4AFED025.1060202@gmail.com>
Date:	Sat, 14 Nov 2009 10:43:33 -0500
From:	William Allen Simpson <william.allen.simpson@...il.com>
To:	Joe Perches <joe@...ches.com>
CC:	Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [net-next-2.6 PATCH v6 3/7 RFC] TCPCT part 1c: sysctl_tcp_cookie_size,
 socket option TCP_COOKIE_TRANSACTIONS

William Allen Simpson wrote:
> Joe Perches wrote:
>> or even adding proc_dointvec_minmax_even
>> might save some cycles during cookie handling.
>>
> Well, that would have to be proc_dointvec_minmax_even_zero(), as the
> valid values can be 0, 8, 10, 12, 14, or 16.  ...
> 
> And it seems to me a case of "premature optimization" -- it might save
> 1 or 2 tests in the order I've listed them in tcp_cookie_size_check(),
> ...
> 
> On the server side, we have to test during parsing anyway.  I never trust
> any data that comes over the network....
> 
I looked at this again today, and proc_dointvec_minmax_even_zero() still
seems to be overkill.  I couldn't find anybody else that does such things
at sysctl parsing time.

I did find we could eliminate a test for odd (in part 1f),

+		if (unlikely(0x1 & cookie_size)) {
+			/* 8-bit multiple, illegal, ignore */
+			cookie_size = 0;

by using a better test for the network data (in part 1g), and it
should be faster, too (assuming gcc is smart enough):

Was:
+				if (TCPOLEN_COOKIE_MAX >= opsize
+				 && TCPOLEN_COOKIE_MIN <= opsize) {

Now:
+				switch (opsize) {
+				case TCPOLEN_COOKIE_MIN+0:
+				case TCPOLEN_COOKIE_MIN+2:
+				case TCPOLEN_COOKIE_MIN+4:
+				case TCPOLEN_COOKIE_MIN+6:
+				case TCPOLEN_COOKIE_MAX:

(This division into so many parts for review is driving me crazy....)

Saved 2 if's for every cookie!  Of course, there's a cpu intensive
SHA1 for each cookie, so this pales in comparison.

Premature optimization or not, thanks for the ideas!

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html