lkml.org 
[lkml]   [2018]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/2] x86/pti: check the return value of pti_user_pagetable_walk_pmd
From
Date
On 07/17/2018 12:23 AM, Jiang Biao wrote:
> Check the return value of pti_user_pagetable_walk_pmd() to avoid
> NULL pointer dereference. And add warning for fail allocation.

For both of these:

Acked-by: Dave Hansen <dave.hansen@intel.com>

It's minor, but if you redo these, I'd appreciate a slightly different
form. Instead of:

> @@ -239,8 +239,10 @@ static pmd_t *pti_user_pagetable_walk_pmd(unsigned long address)
> static __init pte_t *pti_user_pagetable_walk_pte(unsigned long address)
> {
> gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
> - pmd_t *pmd = pti_user_pagetable_walk_pmd(address);
> pte_t *pte;
> + pmd_t *pmd = pti_user_pagetable_walk_pmd(address);
> + if (!pmd)
> + return NULL;

I'd much rather see separation of code -- especially _important_ code
like an allocation -- from local variable definitions. Like this:

gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
pmd_t *pmd;
pte_t *pte;

pmd = pti_user_pagetable_walk_pmd(address);
if (!pmd)
return NULL;

That clearly separtes the variables from the _code_ and also nicely
pairs the action with the check for that action being successful.

\
 
 \ /
  Last update: 2018-07-17 20:03    [W:0.047 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site