linux-kernel - Re: [058/152] tcp: protect sysctl_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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 07 Jan 2011 08:05:09 -0500
From:	William Allen Simpson <william.allen.simpson@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Greg KH <gregkh@...e.de>, linux-kernel@...r.kernel.org,
	stable@...nel.org, stable-review@...nel.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	alan@...rguk.ukuu.org.uk,
	Ben Hutchings <bhutchings@...arflare.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [058/152] tcp: protect sysctl_tcp_cookie_size reads

On 1/7/11 3:30 AM, Eric Dumazet wrote:
> Are you telling us somebody else added bug to your code ?
> This is not the case, obviously.
>
As I noted, "There's been quite a few changes" -- one of which was a major
change to sysctls.  As one of the reviewers of this code, you didn't mention
this ACCESS_ONCE() function, so I'm assuming it didn't exist at that time.

> After bug fix and cleanup code

As I noted, "most of the patch has nothing to do with the purported fix."
That makes the patch hard to read and evaluate.

Moreover, I remember being castigated (by you and others) for combining bug
fixes with cleanup code.  And I'm not sure this counts as "cleanup" code.

> looks good,

As the saying goes, beauty is in the eye of the beholder.  Trading a blank
line for a trailing brace is neither here nor there.  Screen space used
remains the same.

> and even checkpatch.pl is
> fine with it. No need for useless brackets around "return XXX;"

My instructors would have flunked me for not including braces around
multi-line sequences; it was one of the great no-no's of the '70s.  Perhaps
that's not the case anymore with modern colorful visual syntax checkers?

> If I remember well, I did the cleanup so that my patch could not trigger
> checkpatch.pl errors/warnings. Not that I am a particular checkpatch
> fan, but I know some people are.
>
As a relative Linux kernel newbie, I scrupulously followed patch
instructions.  Those instructions mandated running checkpatch.

If there are now "checkpatch.pl errors/warnings" on that code, checkpatch
must have changed.  At the time, all multi-line sequences were required to
be enclosed in braces.

I still think that's a better idea, looks better, and makes maintenance
easier in the long term.  YMMV.

> By the way, you were CCed when I sent one month ago the mail to
> David/netdev. And no reaction from you at that time.
>
Amazingly enough, my life is not centered around Linux on a day-to-day
basis.  Young folks like you are trying to make a name for yourselves in
the Linux world -- apparently, by being exceptionally abrasive.

Your helpful comments are/were appreciated.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/