linux-kernel - Re: [PATCH] add checksum selftest

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: <200906241614.02615.arnd@arndb.de>
Date:	Wed, 24 Jun 2009 16:14:02 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Mike Frysinger <vapier@...too.org>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] add checksum selftest

On Wednesday 24 June 2009, Mike Frysinger wrote:
> Start a checksum internal testsuite for arch porters and people mucking
> about in the checksum code -- regressions are bad.
> 
> Signed-off-by: Mike Frysinger <vapier@...too.org>
> ---
> Arnd: i've tested this on my Blackfin and it seems to work ... can you
> check for any places i missed little endian swapping ?

Endian swapping looks correct, and I like the patch a lot in general,
but:

> +
> +/*
> + * The do_csum() interface is "internal" to the generic checksum code.
> + * Do not require it if the arch has not switched over.
> + */
> +extern unsigned short do_csum(const unsigned char *buff, int len);

Just nitpicking: this prototype should to into asm-generic/checksum.h,
extern declarations have no place in .c files.

> +static unsigned char __initdata do_csum_data1[] = {
> +	0x20,
> +};
> +static unsigned char __initdata do_csum_data2[] = {
> +	0x0d, 0x0a,
> +};
> +static unsigned char __initdata do_csum_data3[] = {
> +	0xff, 0xfb, 0x01,
> +};

You define separate test vectors for each of the three
cases, which looks like it could be optimized by reusing
the same test vectors for each case.

> +static struct csum_partial_data __initdata csum_partial_data[] = {
> +	CSUM_PARTIAL_DATA(1,  0x00000074, 0x0),
> +	CSUM_PARTIAL_DATA(2,  0x00000a0d, 0x0),
> +	CSUM_PARTIAL_DATA(3,  0x0000fe00, 0x0),
> +	CSUM_PARTIAL_DATA(5,  0x00005084, 0x0),
> +	CSUM_PARTIAL_DATA(8,  0x1101eefe, 0x11016a80),
> +	CSUM_PARTIAL_DATA(8b, 0x00008781, 0x847e),
> +	CSUM_PARTIAL_DATA(9,  0x1101eefe, 0x11016b80),
> +};

For partial checksums, the result has to be folded into a 16-bit
number using csum_fold(), because csum_partial and other functions
return a 32-bit __wsum that can take many equivalent values taht
are all correct.

> +static int __init csum_tcpudp_nofold_selftest(void)
> +{
> +	int i, ret;
> +	unsigned short tret, eret;
> +
> +	ret = 0;
> +	for (i = 0; i < ARRAY_SIZE(csum_tcpudp_nofold_data); ++i) {
> +		eret = le16_to_cpu(csum_tcpudp_nofold_data[i].ret);
> +		tret = csum_tcpudp_nofold(
> +			csum_tcpudp_nofold_data[i].saddr,
> +			csum_tcpudp_nofold_data[i].daddr,
> +			csum_tcpudp_nofold_data[i].len,
> +			csum_tcpudp_nofold_data[i].proto,
> +			csum_tcpudp_nofold_data[i].sum);
> +		if (tret != eret) {
> +			pr_err("%s: test %i: %#x != %#x: FAIL\n",
> +				__func__, i, tret, eret);
> +			ret = 1;
> +		}
> +	}
> +
> +	return ret;
> +}

same here, but you can easily use csum_tcpudp_magic() instead of
csum_tcpudp_nofold here.

Thanks,

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