On Mon 19-08-24 14:49:55, David Hildenbrand wrote:

On 19.08.24 14:48, Barry Song wrote:

[...]

quoted

quoted

quoted

quoted

quoted

quoted

quoted

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 60742d057b05..d2c37f8f8d09 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c

@@ -4668,8 +4668,10 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
          * There are several places where we assume that the order value is sane
          * so bail out early if the request is out of bound.
          */
-     if (WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER, gfp))
+     if (WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER, gfp)) {
+             BUG_ON(gfp & __GFP_NOFAIL);
                 return NULL;
+     }

[...]

quoted

Returning NULL doesn't necessarily crash the caller's process, p->field,
*(p + offset) deference could be used by hackers to exploit the system.

See my other reply to Michal: why do we even allow to specify them
separately and not simply let one enforce the other?

Are you replying to this patch? This is not about a combination of
flags. This is about the above (and other similar) boundary checks which
return NULL if the size is deemed incorrect. I think those are potential
problems because it could be a lack of input check which could be turned
into a potentially malicious code. Because unchecked (return value
because NOFAIL never fails, right?) return value might even not OOPs and
become a silent read/write into memory.

Whether to BUG_ON or simply loop for ever in the allocator if somebody
requests non-sleeping NOFAIL allocation is a different story.
-- 
Michal Hocko
SUSE Labs