linux-kernel - Re: [PATCH 1/6] genalloc: track beginning of allocations

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: <947ea9c3-b045-17d3-51e5-df80b4fb27e6@huawei.com>
Date:   Fri, 9 Feb 2018 18:18:06 +0200
From:   Igor Stoppa <igor.stoppa@...wei.com>
To:     Randy Dunlap <rdunlap@...radead.org>, <jglisse@...hat.com>,
        <keescook@...omium.org>, <mhocko@...nel.org>, <labbott@...hat.com>,
        <hch@...radead.org>, <willy@...radead.org>
CC:     <cl@...ux.com>, <linux-security-module@...r.kernel.org>,
        <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
        <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH 1/6] genalloc: track beginning of allocations



On 05/02/18 00:34, Randy Dunlap wrote:
> On 02/04/2018 08:47 AM, Igor Stoppa wrote:

[...]

> It would be good for a lot of this to be in a source file or the
> pmalloc.rst documentation file instead of living only in the git repository.

This is actually about genalloc. The genalloc documentation is high
level and mostly about the API, while this talks about the guts of the
library. The part modified by the patch. This text doesn't seem to
belong to the generic genalloc documentation.
I will move it to the .c file, but isn't it too much text in a source file?

[...]

>> + * @order: pow of 2 represented by each entry in the bitmap
> 
>               power

ok

[...]

>> + * chunk_size - dimension of a chunk of memory
> 
> can this be more explicit about which dimension?

I'll put "size in bytes of a chunk of memory"


[...]

>> + * cleart_bits_ll - according to the mask, clears the bits specified by
> 
>       clear_bits_ll

yes :-(

[...]

>> - * bitmap_clear_ll - clear the specified number of bits at the specified position
>> + * alter_bitmap_ll - set or clear the entries associated to an allocation
> 
>                                                             with an allocation

ok


>> + * @alteration: selection if the bits selected should be set or cleared
> 
>                    indicates if

ok


[...]

>> +	/* Prepare for writing the initial part of the allocation, from
>> +	 * starting entry, to the end of the UL bitmap element which
>> +	 * contains it. It might be larger than the actual allocation.
>> +	 */
> 
> Use kernel multi-line comment style.

ok, also for further occurrences

[...]

>> +	index =  BITS_DIV_LONGS(start_bit);
> 
> 	index = BITS_DIV_LONGS
> (only 1 space after '=')

oops, yes

--
thank you, igor