Re: [PATCH] fscrypt: Replace mk_users keyring with simple list

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Jun 26, 2026 at 09:16:35AM +0100, Luis Henriques wrote:
> Hi Eric!
> 
> On Thu, Jun 18 2026, Eric Biggers wrote:
> 
> > Change mk_users (the set of user claims to an fscrypt master key) from a
> > 'struct key' keyring to a simple linked list.
> >
> > It's still a collection of 'struct key' for quota tracking.  It was
> > originally thought to be natural that a collection of 'struct key'
> > should be held in a 'struct key' keyring.  In reality, it's just been
> > causing problems, similar to how using 'struct key' for the filesystem
> > keyring caused problems and was removed in commit d7e7b9af104c
> > ("fscrypt: stop using keyrings subsystem for fscrypt_master_key").
> >
> > Commit d3a7bd420076 ("fscrypt: clear keyring before calling key_put()")
> > fixed mk_users cleanup to be synchronous.  But that apparently wasn't
> > enough: the keyring subsystem's redundant locking is still generating
> > lockdep false positives due to the interaction with filesystem reclaim.
> >
> > With the simple list, the redundant locking and lockdep issue goes away.
> >
> > Of course, searching a linked list is linear-time whereas the
> > 'struct key' keyring used a fancy constant-time associative array.  But
> > that's fine here, since in practice there's just one entry in the list.
> > In fact the new code is much faster in practice, since it's much smaller
> > and doesn't have to convert the kuid_t into a string to search for it.
> >
> > Reported-by: syzbot+f55b043dacf43776b50c@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f55b043dacf43776b50c
> > Reported-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
> > Closes: https://lore.kernel.org/keyrings/20260614150041.21172-1-med08elkadiri@gmail.com/
> > Fixes: 23c688b54016 ("fscrypt: allow unprivileged users to add/remove keys for v2 policies")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >
> > I'm planning to take this via the fscrypt tree for 7.2
> 
> I was hoping to have some more time to test this patch, but I don't think
> that will happen any time soon.
> 
> I've done a review of the patch and couldn't find any obvious problem.
> Though, again, a more in-depth review would require more time as it has
> been a while since I took a look into this code.
> 
> I just wonder if this is really stable material.  It's a bit intrusive
> (doesn't even apply cleanly in mainline), but above all it's fixing a
> lockdep false positive.  Other than syzbot, has this been seen in the
> wild?

Thanks!

It applies on top of
"[PATCH] fscrypt: Fix key setup in edge case with multiple data unit sizes"
(https://lore.kernel.org/linux-fscrypt/20260618180652.52742-1-ebiggers@kernel.org/).
This time I tried just relying on the prerequisite-patch-id footer (as
generated by 'git format-patch') to express the dependency.  But
evidently that still doesn't work: for one, 'b4 am' just ignores it.

Since this patch has "Reported-by: syzbot" as well as "Fixes", the
stable maintainers will apply it anyway.  If I actually wanted to stop
that, I'd have to actively oppose the backport, likely multiple times
indefinitely since people will continue to try to backport it.  And
syzkaller would continue to get the lockdep warning on stable kernels.

So I'd rather just get it out the way and backport it the same time as
"fscrypt: Fix key setup in edge case with multiple data unit sizes",
which similarly tweaks some data structures in struct fscrypt_master_key
and needs to be backported too.  "fscrypt: stop using keyrings subsystem
for fscrypt_master_key" several years ago was backported too.

- Eric