netdev - Re: [PATCH 05/21] RDS: Info and stats

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: <20090127132759.GG2646@ioremap.net>
Date:	Tue, 27 Jan 2009 16:28:00 +0300
From:	Evgeniy Polyakov <zbr@...emap.net>
To:	Andy Grover <andy.grover@...cle.com>
Cc:	rdreier@...co.com, rds-devel@....oracle.com,
	general@...ts.openfabrics.org, netdev@...r.kernel.org
Subject: Re: [PATCH 05/21] RDS: Info and stats

On Mon, Jan 26, 2009 at 06:17:42PM -0800, Andy Grover (andy.grover@...cle.com) wrote:
> +void rds_info_register_func(int optname, rds_info_func func)
> +{
> +	int offset = optname - RDS_INFO_FIRST;
> +
> +	BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST);
> +
> +	spin_lock(&rds_info_lock);
> +	BUG_ON(rds_info_funcs[offset] != NULL);
> +	rds_info_funcs[offset] = func;
> +	spin_unlock(&rds_info_lock);
> +}
> +EXPORT_SYMBOL_GPL(rds_info_register_func);
> +
> +void rds_info_deregister_func(int optname, rds_info_func func)
> +{
> +	int offset = optname - RDS_INFO_FIRST;
> +
> +	BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST);
> +

Those bug_ons look quite scary, is there a way to actually have a wrong
optname? Plus, those _INFO definitions are declared twice in the code,
which makes it harder to update.

> +/*
> + * Typically we hold an atomic kmap across multiple rds_info_copy() calls
> + * because the kmap is so expensive.  This must be called before using blocking
> + * operations while holding the mapping and as the iterator is torn down.
> + */
> +void rds_info_iter_unmap(struct rds_info_iterator *iter)
> +{
> +	if (iter->addr != NULL) {
> +		kunmap_atomic(iter->addr, KM_USER0);
> +		iter->addr = NULL;
> +	}
> +}
> +

This one is used to temporarily map some address, but functions called
between map and unmap functions (like rds_info_getsockopt()) may sleep,
which is wrong.

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