Hi David, Thanks for your review. > > mmap() returns MAP_FAILED, which is defined as (void *)-1, on error, > > not NULL. Several selftests incorrectly check the return value of > > mmap() using !ptr or ptr == NULL, which would erroneously treat > > MAP_FAILED as a valid pointer since MAP_FAILED is non-zero and > > non-NULL. This can lead to segfaults when mmap() actually fails > > under memory pressure. > > Well, your patch also adds more checks where we previously didn't have any > checks?
Yes, I added a few checks for the mmap return value, as they were not handled previously. Perhaps I should also adjust the commit message to match this change. > > > > Signed-off-by: Hongfu Li <[email protected]> > > Reviewed-by: Dev Jain <[email protected]> > > --- > > v2: > > - Add missing mmap() return value checks in pkey_sighandler_tests.c > > and protection_keys.c > > --- > > [...] > > > diff --git a/tools/testing/selftests/mm/protection_keys.c > > b/tools/testing/selftests/mm/protection_keys.c > > index 2085982dba69..580bf1668c71 100644 > > --- a/tools/testing/selftests/mm/protection_keys.c > > +++ b/tools/testing/selftests/mm/protection_keys.c > > @@ -1217,6 +1217,7 @@ static void arch_force_pkey_reg_init(void) > > * doing the XSAVE size enumeration dance. > > */ > > buf = mmap(NULL, 1*MB, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, > > -1, 0); > > + pkey_assert(buf != (void *)-1); > > What's the reason for not using MAP_FAILED? I tried to keep consistency with the existing mmap handling pattern in protection_keys.c. That said, using MAP_FAILED would indeed be more appropriate. I'll adjust this and send out a v3 shortly. Best regards, Hongfu
