Re: [PATCH RFC] mm: warn potential return NULL for kmalloc_array and kvmalloc_array with __GFP_NOFAIL

[Michal Hocko <mhocko@suse.com>]

On Thu 18-07-24 19:41:33, Barry Song wrote:
> On Thu, Jul 18, 2024 at 7:27 PM Michal Hocko <mhocko@suse.com> wrote:
> >
> > On Thu 18-07-24 19:22:37, Barry Song wrote:
> > [...]
> > > For future-proofing and security reasons, returning NULL for NOFAIL
> > > still seems incorrect as the callers won't check the ret. If any future or
> > > existing in-tree code has a potential bug which might be exploited by
> > > hackers, for example
> > >
> > > ptr = kvmalloc_array(NOFAIL);
> > > ptr->callback(); //ptr=NULL;
> > >
> > > callback could be a privilege escalation?
> >
> > Only if you allow to map zero page AFAIK. Nobody reasonable should be
> > doing that.
> 
> ptr->callback could be above /proc/sys/vm/mmap_min_addr ?

Yes, it can of course but this would require quite a stretch to trigger,
no?

Look at this from a real life code POV. You are allocating an array of
callbacks (or structure of callbacks). In order to have this exploitable
you need to direct the first dereference above mmap_min_addr.

If you really want to protect from a code like that then WARN_ON doesn't
buy you anything because it will stop the exploit only when
panic_on_warn. You would need BUG_ON as mentioned by Christoph.

So the real question is, do you want to stop exploits or do you want to
debug potentially incorrect but mostly harmless buggy code?
-- 
Michal Hocko
SUSE Labs