netdev - Re: [RFC] [Patch 2/4] dccp: Lockless use of CCID blocks

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: <20081223171729.GB3855@gerrit.erg.abdn.ac.uk>
Date:	Tue, 23 Dec 2008 18:17:29 +0100
From:	Gerrit Renker <gerrit@....abdn.ac.uk>
To:	Arnaldo Carvalho de Melo <acme@...hat.com>, dccp@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [RFC] [Patch 2/4] dccp: Lockless use of CCID blocks

| > -int ccid_unregister(struct ccid_operations *ccid_ops)
| > +static int ccid_unregister(struct ccid_operations *ccid_ops)
| >  {
<snip>
| 
| And "register/unregister" now don't make much sense since there is no
| registration being done, just allocating/deallocating resources
| 
Yes agree - this needs revision. We can also drop the EXPORT_SYMBOLS
from the now-static routines.

| > +
| > +static struct ccid_operations *ccid_find_by_id(const u8 id)
| >  {
| > -	struct ccid *ccid = kmem_cache_alloc(rx ? ccid_ops->ccid_hc_rx_slab :
| > -						  ccid_ops->ccid_hc_tx_slab, gfp);
| > +	int i;
| > +
| > +	for (i = 0; i < ARRAY_SIZE(ccids); i++)
| > +		if (ccids[i]->ccid_id == id)
| > +			return ccids[i];
| > +	return NULL;
| 
| Why the we searching? Can't we just do:
| 
| {
| 	if (id > ARRAY_SIZE(ccids) - 2)
| 		return NULL;
| 	return ccids[id - 2];
| }
| 
Agree 100%, I don't like the routine myself and have been thinking about
how to rewrite it. Current idea is to go back to the original and use

static struct ccid_operations *ccids[CCIDS_MAX] = {
	[DCCPC_CCID2] = &ccid2_ops,
#ifdef CONFIG_IP_DCCP_CCID3
	[DCCPC_CCID3] = &ccid3_ops,
#endif
};

And then use 
	
	if (id < 0 || id >= CCIDS_MAX)
		return NULL;
	return ccids[id];		

which may be NULL if there is no entry in the array. Better?	

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