netdev - Re: [PATCHv6] net: Add batman-adv meshing protocol

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: <20101201.104314.112592938.davem@davemloft.net>
Date:	Wed, 01 Dec 2010 10:43:14 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	sven.eckelmann@....de
Cc:	netdev@...r.kernel.org, b.a.t.m.a.n@...ts.open-mesh.org
Subject: Re: [PATCHv6] net: Add batman-adv meshing protocol

From: Sven Eckelmann <sven.eckelmann@....de>
Date: Sun,  7 Nov 2010 14:26:17 +0100

> +		if (seq_bits[word_num] & 1 << word_offset)
 ...
> +	seq_bits[word_num] |= 1 << word_offset;	/* turn the position on */
 ...
> +#define TYPE_OF_WORD unsigned long

"1" is an 'int' and won't get promoted to unsigned long which means
this code won't work on 64-bit, you need to explicitly say "1UL".

This seems to duplicate a lot of existing functionality, we have
properly bit set/clear/change/test routines, that operate on
"unsigned long" just as your code does.

And since you don't use the existing facilities, you end up with
bugs like this one.  Bug which are completely unnecessary.

The badman-adv code is full of duplicated functionality, and until
all of these cases are cured I refuse to integrate this code.  I
already complained about the hashing stuff, and now there's this
stuff too.

Every time I review the batman-adv code I find a bug, and the bug is
often in facilities which the kernel has already and are being
duplicated.  That is by definition a waste of my and everyone else's
time.

Probably your submission will be almost half the size that it is
currently once you take care of this issue. :-)

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