On Mon 19-08-24 14:49:55, David Hildenbrand wrote:
> On 19.08.24 14:48, Barry Song wrote:
[...]
> > > > > > > > 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;
> > > > > > > > + }
[...]
> > 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
