Re: [PATCH v2] mm_access: simplify the security checks

[Oleg Nesterov]

On 06/01, Lorenzo Stoakes wrote:
>
> +cc Liam for mm lifecycle stuff :)
>
> The subject here seems not quite right - you're adding complexity here in that
> now there's a racey fast path.

OK. See my reply to David. If it doesn't look like a simplification -
lets forget this patch ;)

> One behavioural change here though is that down_read_killable() was used
> previously, so such a situation would return -EINTR, but now would instead
> succeed.

I don't really follow... SIGKILL from de_thread() or anything else can
come right after down_read_killable().

> >    All we need for correctness is READ_ONCE() to ensure the compiler
> >    won't reload task->mm. This is not enough for KCSAN, but we already
>
> I'm not sure 'this is not enough for KCSAN' is really reassuring :)

If I understand correctly KCSAN will complain if (say) we race with the
exiting task which does current->mm = NULL without WRITE_ONCE in exit_mm().

> It's useful to put a revision history (ideally with links to prior revisions)
> below the --- line to explain how vN differs from v(N-1).

Yes... I didn't do it this time because V2 doesn't differ from V1, I just removed
the duplicated paragraph from the changelog.

> Overall I'm not really convinced about this patch - this isn't simplifying
> things, it's introducing subtle assumptions and I don't really see the
> benefit?

Thanks for review! lets forget this patch then.

Oleg.