Re: [PATCH v3] misc: sgi-gru: fix use-after-free error in gru_set_context_option, gru_fault and gru_handle_user_call_os

From: Zheng Hacker
Date: Tue Oct 18 2022 - 22:39:32 EST


Zheng Yejian <zhengyejian1@xxxxxxxxxx> 于2022年10月19日周三 09:56写道:
>
> On Wed, 19 Oct 2022 00:52:59 +0800, Zheng Wang <zyytlz.wz@xxxxxxx> wrote:
> > Gts may be freed in gru_check_chiplet_assignment.
> > The caller still use it after that, UAF happens.
> >
> > Fix it by introducing a return value to see if it's in error path or not.
> > Free the gts in caller if gru_check_chiplet_assignment check failed.
> >
> > Fixes: 55484c45dbec ("gru: allow users to specify gru chiplet 2")
> > Reported-by: Zheng Wang <hackerzheng666@xxxxxxxxx>
> > Signed-off-by: Zheng Wang <zyytlz.wz@xxxxxxx>
> > ---
> > v3:
> > - add preempt_enable and use VM_FAULT_NOPAGE as failure code by Yejian
>
> Sorry, maybe I didn't make it clearly enough in
> https://lore.kernel.org/lkml/20221009201418.509417-1-zhengyejian1@xxxxxxxxxx/
>
> What I suggested was that return value of gru_fault() should be some VM_FAULT_XXX,
> because this return value would be specially handled in pagefault flow like:
> __do_fault() {
> ......
> ret = vma->vm_ops->fault(vmf);
> ......
> }
>
> But in other functions that are not in pagefault flow, common error code
> like -EINVAL is ok. Meanings of every error code can be found in:
> https://man7.org/linux/man-pages/man3/errno.3.html
>
> By the way, these are all personal suggestions. You need Reviewed-by Maintainer :)

Hi Zheng Yejian,

Thanks for your suggestion! Sorry for my late patch, I was a little
bit busy last week. I think the trigger path of this issue is divise.
So yes, -EINVAL may be a better choice.

I'll update the patch soon after.

Best Regards,
Zheng Wang