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

[Zheng Yejian]

On Wed, 19 Oct 2022 10:39:11 +0800, Zheng Hacker <hackerzheng666@xxxxxxxxx> wrote:
> 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.

Hi Wang,
I'm very glad for my suggestion being adopted.
And really thank you for your efforts!

>
> Best Regards,
> Zheng Wang