linux-kernel - Re: [PATCH v5 1/7] mm: pass task and mm to do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 14 Feb 2020 18:25:30 +0100
From:   Jann Horn <jannh@...gle.com>
To:     Minchan Kim <minchan@...nel.org>, Jens Axboe <axboe@...nel.dk>,
        io-uring <io-uring@...r.kernel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>,
        Linux API <linux-api@...r.kernel.org>,
        Oleksandr Natalenko <oleksandr@...hat.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Tim Murray <timmurray@...gle.com>,
        Daniel Colascione <dancol@...gle.com>,
        Sandeep Patil <sspatil@...gle.com>,
        Sonny Rao <sonnyrao@...gle.com>,
        Brian Geffon <bgeffon@...gle.com>,
        Michal Hocko <mhocko@...e.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Shakeel Butt <shakeelb@...gle.com>,
        John Dias <joaodias@...gle.com>,
        Joel Fernandes <joel@...lfernandes.org>, sj38.park@...il.com,
        Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Subject: Re: [PATCH v5 1/7] mm: pass task and mm to do_madvise

+Jens and io-uring list

On Fri, Feb 14, 2020 at 6:06 PM Minchan Kim <minchan@...nel.org> wrote:
> In upcoming patches, do_madvise will be called from external process
> context so we shouldn't asssume "current" is always hinted process's
> task_struct.
[...]
> [1] http://lore.kernel.org/r/CAG48ez27=pwm5m_N_988xT1huO7g7h6arTQL44zev6TD-h-7Tg@mail.gmail.com
[...]
> diff --git a/fs/io_uring.c b/fs/io_uring.c
[...]
> @@ -2736,7 +2736,7 @@ static int io_madvise(struct io_kiocb *req, struct io_kiocb **nxt,
>         if (force_nonblock)
>                 return -EAGAIN;
>
> -       ret = do_madvise(ma->addr, ma->len, ma->advice);
> +       ret = do_madvise(current, current->mm, ma->addr, ma->len, ma->advice);
>         if (ret < 0)
>                 req_set_fail_links(req);
>         io_cqring_add_event(req, ret);

Jens, can you have a look at this change and the following patch
<https://lore.kernel.org/linux-mm/20200214170520.160271-4-minchan@kernel.org/>
("[PATCH v5 3/7] mm: check fatal signal pending of target process")?
Basically Minchan's patch tries to plumb through the identity of the
target task so that if that task gets killed in the middle of the
operation, the (potentially long-running and costly) madvise operation
can be cancelled. Just passing in "current" instead (which in this
case is the uring worker thread AFAIK) doesn't really break anything,
other than making the optimization not work, but I wonder whether this
couldn't be done more cleanly - maybe by passing in NULL to mean "we
don't know who the target task is", since I think we don't know that
here?