Messages in this thread | | | Subject | Re: [PATCH v2 2/2] x86/pti: check the return value of pti_user_pagetable_walk_pmd | From | Dave Hansen <> | Date | Tue, 17 Jul 2018 11:03:39 -0700 |
| |
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.
| |