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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 06 Nov 2013 15:46:42 -0800
From:	Olav Haugan <ohaugan@...eaurora.org>
To:	Nitin Gupta <ngupta@...are.org>,
	David Cohen <david.a.cohen@...ux.intel.com>
CC:	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 11/6/2013 12:56 PM, Nitin Gupta wrote:
> 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.
> 

Sure, I can add a comment above obj_location_to_handle() and
obj_handle_to_location().


Olav Haugan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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/