On Thu 18-07-24 19:41:33, Barry Song wrote: > On Thu, Jul 18, 2024 at 7:27 PM Michal Hocko <mhocko@xxxxxxxx> 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
