linux-kernel - Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPkvG_ftpPJMdYdOmKdYT2e6sd+tntX_vrvpLbAW85dXYJiQdw@mail.gmail.com>
Date:	Wed, 6 Nov 2013 12:56:36 -0800
From:	Nitin Gupta <ngupta@...are.org>
To:	David Cohen <david.a.cohen@...ux.intel.com>
Cc:	Olav Haugan <ohaugan@...eaurora.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Seth Jennings <sjenning@...ux.vnet.ibm.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Minchan Kim <minchan@...nel.org>, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success

On Tue, Nov 5, 2013 at 5:17 PM, David Cohen
<david.a.cohen@...ux.intel.com> wrote:
> Hi Olav,
>
>
> On 11/05/2013 04:54 PM, Olav Haugan wrote:
>>
>> zsmalloc encodes a handle using the page pfn and an object
>> index. On some hardware platforms the pfn could be 0 and this
>> causes the encoded handle to be 0 which is interpreted as an
>> allocation failure.
>>
>> To prevent this false error we ensure that the encoded handle
>> will not be 0 when allocation succeeds.
>>
>> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6
>> Signed-off-by: Olav Haugan <ohaugan@...eaurora.org>
>> ---
>>   drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c
>> b/drivers/staging/zsmalloc/zsmalloc-main.c
>> index 523b937..0e32c0f 100644
>> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
>> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
>> @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page,
>> unsigned long obj_idx)
>>         }
>>
>>         handle = page_to_pfn(page) << OBJ_INDEX_BITS;
>> -       handle |= (obj_idx & OBJ_INDEX_MASK);
>> +       handle |= ((obj_idx + 1) & OBJ_INDEX_MASK);
>
>
> As suggestion you could use a macro instead of hardcoded 1.
>
> I am not familiar with this code, but if it's a valid test to verify if
> the resulting address is page aligned, you might want to set this
> offset macro to a page aligned value as well.
>
>

Using a hardcoded 1 looks fine in this case. But the patch description
should also be added as a comment for this function. Otherwise, the patch
looks good to me.

>>
>>         return (void *)handle;
>>   }
>> @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long
>> handle, struct page **page,
>>                                 unsigned long *obj_idx)
>>   {
>>         *page = pfn_to_page(handle >> OBJ_INDEX_BITS);
>> -       *obj_idx = handle & OBJ_INDEX_MASK;
>> +       *obj_idx = (handle & OBJ_INDEX_MASK) - 1;
>
>
> Ditto.
>

Thanks,
Nitin
--
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/