Hi1
> >>>> --- a/mm/page_alloc.c
> >>>> +++ b/mm/page_alloc.c
> >>>> @@ -4364,6 +4353,15 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned
> >>>> int order, int preferred_nid,
> >>>> gfp_t alloc_mask; /* The gfp_t that was actually used for
> >>>> allocation */
> >>>> struct alloc_context ac = { };
> >>>>
> >>>> + /*
> >>>> + * 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 (unlikely(order >= MAX_ORDER)) {
> >>>> + WARN_ON_ONCE(!(gfp_mask & __GFP_NOWARN));
> >>>> + return NULL;
> >>>> + }
> >>>> +
> >>>
> >>> I know "everybody enables CONFIG_DEBUG_VM", but given this is fastpath,
> >>> we could help those who choose not to enable it by using
> >>>
> >>> #ifdef CONFIG_DEBUG_VM
> >>> if (WARN_ON_ONCE(order >= MAX_ORDER && !(gfp_mask & __GFP_NOWARN)))
> >>> return NULL;
> >>> #endif
> >>
> >> Hmm, but that would mean there's still potential undefined behavior for
> >> !CONFIG_DEBUG_VM, so I would prefer not to do it like that.
> >>
> >
> > What does "potential undefined behavior" mean here?
>
> I mean that it becomes undefined once a caller with order >= MAX_ORDER
> appears. Worse if it's directly due to a userspace action, like in this
> case.
We should really check if value from userspace is sane _before_
passing it to alloc_pages(). Anything else is too fragile. Maybe
alloc_pages should do the second check, but...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
