lkml.org 
[lkml]   [2019]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: KASAN: use-after-free Read in get_mem_cgroup_from_mm
    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@huawei.com> wrote:
    > >>>>>> On 2019/3/4 22:11, Dmitry Vyukov wrote:
    > >>>>>>> On Mon, Mar 4, 2019 at 3:00 PM zhong jiang <zhongjiang@huawei.com> wrote:
    > >>>>>>>> On 2019/3/4 15:40, Dmitry Vyukov wrote:
    > >>>>>>>>> On Sun, Mar 3, 2019 at 5:19 PM zhong jiang <zhongjiang@huawei.com> 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

    \
     
     \ /
      Last update: 2019-03-06 09:13    [W:5.378 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site