linux-kernel - Re: [GIT]: Networking

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: <alpine.LFD.2.01.0906161155400.3282@localhost.localdomain>
Date:	Tue, 16 Jun 2009 11:59:34 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Eric Dumazet <eric.dumazet@...il.com>
cc:	David Miller <davem@...emloft.net>, mingo@...e.hu,
	alan@...rguk.ukuu.org.uk, akpm@...ux-foundation.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT]: Networking

On Tue, 16 Jun 2009, Eric Dumazet wrote:
> 
> Here is first patch to take into account this initial offset in sk_wmem_alloc
> 
> (Only compiled, not tested)

I think this is incredibly ugly and hacky.

> @@ -162,7 +162,7 @@ static void atalk_destroy_timer(unsigned long data)
>  {
>  	struct sock *sk = (struct sock *)data;
>  
> -	if (atomic_read(&sk->sk_wmem_alloc) ||
> +	if (atomic_read(&sk->sk_wmem_alloc) > 1 ||
>  	    atomic_read(&sk->sk_rmem_alloc)) {

Seriously, look at that code, and tell me it makes sense.

No, it does not. The code looks like totally random line noise, and that 
whole "> 1" test makes no conceptual sense what-so-ever.

It _will_ result in random bugs later on, because code that doesn't make 
sense will never be good in the long run.

At the very least, add a helper function for "do I actually have 
outstanding allocations" or something like that. IOW, do a 

	/*
	 * Comment here about that magical "1"
	 */
	static inline int sk_has_allocations(struct sock *sk)
	{
		return atomic_read(&sk->sk_wmem_alloc) > 1 ||
			atomic_read(&sk->sk_rmem_alloc);
	}

and then make the various network protocols use that, rather than 
open-coding some random internal implementation magic.

		Linus
--
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/