Re: [PATCH] mm/secretmem: Avoid letting secretmem_users drop to zero

[Kees Cook <keescook@chromium.org>]

On Thu, Oct 21, 2021 at 07:53:11PM -0700, Andrew Morton wrote:
> On Thu, 21 Oct 2021 08:40:46 -0700 Kees Cook <keescook@chromium.org> wrote:
> 
> > Quoting Dmitry: "refcount_inc() needs to be done before fd_install().
> > After fd_install() finishes, the fd can be used by userspace and we can
> > have secret data in memory before the refcount_inc().
> > 
> > A straightforward mis-use where a user will predict the returned fd
> > in another thread before the syscall returns and will use it to store
> > secret data is somewhat dubious because such a user just shoots themself
> > in the foot.
> > 
> > But a more interesting mis-use would be to close the predicted fd and
> > decrement the refcount before the corresponding refcount_inc, this way
> > one can briefly drop the refcount to zero while there are other users
> > of secretmem."
> > 
> > Move fd_install() after refcount_inc().
> 
> I added cc:stable.  Or doesn't the benefit/risk ratio justify that?

I hadn't because commit 110860541f44 ("mm/secretmem: use refcount_t
instead of atomic_t") wasn't, and this would build on top of it.

I think the exposure is very small in both places, so probably best to
avoid the churn, but I'm not _opposed_ to it.

-- 
Kees Cook