On Wed, Mar 06, 2019 at 03:41:06PM +0800, zhong jiang wrote: > On 2019/3/6 14:26, Mike Rapoport wrote: > > Hi, > > > > On Wed, Mar 06, 2019 at 01:53:12PM +0800, zhong jiang wrote: > >> On 2019/3/6 10:05, Andrea Arcangeli wrote: > >>> Hello everyone, > >>> > >>> [ CC'ed Mike and Peter ] > >>> > >>> On Tue, Mar 05, 2019 at 02:42:00PM +0800, zhong jiang wrote: > >>>> On 2019/3/5 14:26, Dmitry Vyukov wrote: > >>>>> On Mon, Mar 4, 2019 at 4:32 PM zhong jiang <zhongjiang@xxxxxxxxxx> wrote: > >>>>>> On 2019/3/4 22:11, Dmitry Vyukov wrote: > >>>>>>> On Mon, Mar 4, 2019 at 3:00 PM zhong jiang <zhongjiang@xxxxxxxxxx> wrote: > >>>>>>>> On 2019/3/4 15:40, Dmitry Vyukov wrote: > >>>>>>>>> On Sun, Mar 3, 2019 at 5:19 PM zhong jiang <zhongjiang@xxxxxxxxxx> wrote: > >>>>>>>>>> Hi, guys > >>>>>>>>>> > >>>>>>>>>> I also hit the following issue. but it fails to reproduce the issue by the log. > >>>>>>>>>> > >>>>>>>>>> it seems to the case that we access the mm->owner and deference it will result in the UAF. > >>>>>>>>>> But it should not be possible that we specify the incomplete process to be the mm->owner. > >>>>>>>>>> > >>>>>>>>>> Any thoughts? > >>>>>>>>> FWIW syzbot was able to reproduce this with this reproducer. > >>>>>>>>> This looks like a very subtle race (threaded reproducer that runs > >>>>>>>>> repeatedly in multiple processes), so most likely we are looking for > >>>>>>>>> something like few instructions inconsistency window. > >>>>>>>>> > >>>>>>>> I has a little doubtful about the instrustions inconsistency window. > >>>>>>>> > >>>>>>>> I guess that you mean some smb barriers should be taken into account.:-) > >>>>>>>> > >>>>>>>> Because IMO, It should not be the lock case to result in the issue. > >>>>>>> Since the crash was triggered on x86 _most likley_ this is not a > >>>>>>> missed barrier. What I meant is that one thread needs to executed some > >>>>>>> code, while another thread is stopped within few instructions. > >>>>>>> > >>>>>>> > >>>>>> It is weird and I can not find any relationship you had said with the issue.:-( > >>>>>> > >>>>>> Because It is the cause that mm->owner has been freed, whereas we still deference it. > >>>>>> > >>>>>> From the lastest freed task call trace, It fails to create process. > >>>>>> > >>>>>> Am I miss something or I misunderstand your meaning. Please correct me. > >>>>> Your analysis looks correct. I am just saying that the root cause of > >>>>> this use-after-free seems to be a race condition. > >>>>> > >>>>> > >>>>> > >>>> Yep, Indeed, I can not figure out how the race works. I will dig up further. > >>> Yes it's a race condition. > >>> > >>> We were aware about the non-cooperative fork userfaultfd feature > >>> creating userfaultfd file descriptor that gets reported to the parent > >>> uffd, despite they belong to mm created by failed forks. > >>> > >>> https://www.spinics.net/lists/linux-mm/msg136357.html > >>> > >> Hi, Andrea > >> > >> I still not clear why uffd ioctl can use the incomplete process as the mm->owner. > >> and how to produce the race. > > There is a C reproducer in the syzcaller report: > > > > https://syzkaller.appspot.com/x/repro.c?x=172fa5a3400000 > > > >> From your above explainations, My underdtanding is that the process handling do_exexve > >> will have a temporary mm, which will be used by the UUFD ioctl. > > The race is between userfaultfd operation and fork() failure: > > > > forking thread | userfaultfd monitor thread > > --------------------------------+------------------------------- > > fork() | > > dup_mmap() | > > dup_userfaultfd() | > > dup_userfaultfd_complete() | > > | read(UFFD_EVENT_FORK) > > | uffdio_copy() > > | mmget_not_zero() > > goto bad_fork_something | > > ... | > > bad_fork_free: | > > free_task() | > > | mem_cgroup_from_task() > > | /* access stale mm->owner */ > > > Hi, Mike Hi, Zhong, > > forking thread fails to create the process ,and then free the allocated task struct. > Other userfaultfd monitor thread should not access the stale mm->owner. > > The parent process and child process do not share the mm struct. Userfaultfd monitor thread's > mm->owner should not point to the freed child task_struct. IIUC the problem is that above mm (of the mm->owner) is the child process's mm rather than the uffd monitor's. When dup_userfaultfd_complete() is called there will be a new userfaultfd context sent to the uffd monitor thread which linked to the chlid process's mm, and if the monitor thread do UFFDIO_COPY upon the newly received userfaultfd it'll operate on that new mm too. > > and due to the existence of tasklist_lock, we can not specify the mm->owner to freed task_struct. > > I miss something,=-O > > Thanks, > zhong jiang > >> Thanks, > >> zhong jiang > > Regards, -- Peter Xu
