linux-kernel - Re: [RFC v2] genalloc:add an gen_pool_first_fit

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: <1438032043.2993.311.camel@freescale.com>
Date:	Mon, 27 Jul 2015 16:20:43 -0500
From:	Scott Wood <scottwood@...escale.com>
To:	Zhao Qiang <qiang.zhao@...escale.com>
CC:	<lauraa@...eaurora.org>, <linux-kernel@...r.kernel.org>,
	<linuxppc-dev@...ts.ozlabs.org>, <akpm@...ux-foundation.org>,
	<olof@...om.net>, <catalin.marinas@....com>, <X.xie@...escale.com>
Subject: Re: [RFC v2] genalloc:add an gen_pool_first_fit_align algo to
 genalloc

On Mon, 2015-07-27 at 17:57 +0800, Zhao Qiang wrote:
> diff --git a/lib/genalloc.c b/lib/genalloc.c
> index d214866..e6608cd 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -509,6 +509,31 @@ unsigned long gen_pool_first_fit(unsigned long *map, 
> unsigned long size,
>  EXPORT_SYMBOL(gen_pool_first_fit);
>  
>  /**
> + * gen_pool_first_fit_align - find the first available region
> + * of memory matching the size requirement (no alignment constraint)
> + * @map: The address to base the search on
> + * @size: The bitmap size in bits
> + * @start: The bitnumber to start searching at
> + * @nr: The number of zeroed bits we're looking for
> + * @data: additional data - unused
> + */
> +unsigned long gen_pool_first_fit_align(unsigned long *map, unsigned long 
> size,
> +             unsigned long start, unsigned int nr, void *data)
> +{
> +     unsigned long align_mask;
> +     int order;
> +
> +     if (data && data->pool) {

There is no way that this compiles.  You can't dereference a void pointer.

Please test your code before submitting, even for an RFC.

> +             order = data->pool->min_alloc_order;

I don't think pool belongs in data.  It's fundamental enough that, if a 
pointer to pool is needed, it should be an argument to the algorithm.

> +             align_mask = ((data->align + (1UL << order) - 1) >> order) - 1;
> +     } else {
> +             pr_err("no data or data->pool\n");
> +     }

This is way too vague and unobtrusive of an error message, and also not rate-
limited, etc.  I wouldn't bother checking at all.  Just let it crash on the 
developer's machine if they use this without passing in data.

Where's the part that adds the ability to pass in data to each allocation 
call, as per the previous discussion?

-Scott

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