On Fri, Jun 26, 2026 at 07:02:32PM +0000, Eric Biggers wrote:
> 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: [email protected]
> > > Closes: https://syzkaller.appspot.com/bug?extid=f55b043dacf43776b50c
> > > Reported-by: Mohammed EL Kadiri <[email protected]>
> > > Closes:
> > > https://lore.kernel.org/keyrings/[email protected]/
> > > Fixes: 23c688b54016 ("fscrypt: allow unprivileged users to add/remove
> > > keys for v2 policies")
> > > Cc: [email protected]
> > > Signed-off-by: Eric Biggers <[email protected]>
> > > ---
> > >
> > > 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/[email protected]/).
> 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.
FWIW, I would also not be surprised if the old code would also fail
fuzzing in other ways, like keyctl() being used to directly manipulate
the keyrings from underneath what fs/crypto/ assumes. I remember at
least considering that scenario when adding this code years ago, but I
think the reasoning was quite subtle and I may have missed something.
The 'struct key' keyrings just have a lot of obscure sharp corners.
Whereas simple lists, hash tables, etc. are much easier to evaluate.
- Eric
