Re: [PATCH v4 0/2] keys: Fix quota-owner reads and ownership accounting

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Oct 13, 2026 at 04:40:54PM +0800, Chengfeng Ye wrote:
> This revision keeps the v3 topic order: the lifetime/namespace-reader fix
> is patch 1, and ownership accounting is patch 2. Each patch can be applied
> independently to the base, and both orders produce the same final tree.
> 
> Patch 1 protects only the three quota-owner UID snapshots. It relies on
> key_user_lock already held by lookup and final reference removal. Its
> READ_ONCE() pointer loads pair with a WRITE_ONCE() replacement in chown.
> The reference-release ordering and lock handoff keep a sampled owner alive;
> this does not require patch 2's new accounting critical section. The commit
> message gives the lifetime argument, including a final put by another task.
> 
> Patch 2 serializes chown with quota reservation and positive and negative
> instantiation. Each critical section is complete within this commit.
> Atomic counter operations alone do not serialize owner selection and
> count/state publication with chown, and instantiation does not require the
> target key's semaphore.
> 
> The messages explain why keeping a key alive does not pin its former
> quota owner, and why key->sem does not exclude instantiation. They map
> those two requirements to the three reader scopes and four accounting
> scopes, and explain the existing protection at unchanged lifecycle sites.
> All seven scopes in the combined v3 series are retained. Reservation and
> instantiation locking move from v3 1/2 into the complete accounting fix
> in v4 2/2; no lock scope is split between commits.
> 
> The procfs changes cover the originally reported proc_keys_start() fault.
> The filters continue to use the quota-owner UID, since key_fsuid_changed()
> can change a thread keyring's key->uid without moving its quota owner.
> 
> Changes in v4:
> - Keep the v3 topic order: lifetime/namespace readers in patch 1,
>   accounting in patch 2. Both fixes remain independently applicable.
> - Put every accounting critical section wholly in patch 2.
> - Make patch 1 independent using existing lifetime locking plus explicit
>   pointer access marking.
> - Explain the root cause and every critical section, plus the existing
>   protection at unchanged sites. Keep allocation, notifications and
>   reference release outside the new accounting scopes.
> - Correct the reader patch's Fixes tags to the namespace-filter commits.
> - Rebase onto for-next-keys at 22430ae5d90ab and request fresh review.
> 
> Validation:
> - Built security/keys/ with patch 1 only, patch 2 only, and both patches.
>   KEYS, PROC_FS, USER_NS, KEY_NOTIFICATIONS, BIG_KEYS, ENCRYPTED_KEYS and
>   PROVE_LOCKING were enabled. No compiler warnings or errors.
> - Both exported patches pass checkpatch --strict with zero diagnostics.
> - Both patches apply directly to the base. Ordered git am in either order
>   reproduces the expected intermediate and final trees.
> - No new runtime, lockdep or Sashiko run. The KASAN excerpt is historical.
> 
> OpenAI Codex assisted with the review audit, series split, pointer access
> marking, commit messages and build checks. The existing author sign-off
> is retained from v3 for human review before submission.
> 
> Previous revisions:
> v3: https://lore.kernel.org/r/20260927162528.943886-1-nicoyip.dev@gmail.com/
> v2: https://lore.kernel.org/r/20260904080940.575882-1-nicoyip.dev@gmail.com/
> v1: https://lore.kernel.org/r/20260823170448.3856516-1-nicoyip.dev@gmail.com/
> Review:
> https://lore.kernel.org/r/asL2UQjYCxt9SF9K@kernel.org/
> https://lore.kernel.org/r/asMQMzqVkmYhwgMB@kernel.org/
> https://lore.kernel.org/r/asNCIs3OshLwGWrB@kernel.org/
> 
> Chengfeng Ye (2):
>   keys: Protect quota-owner reads in namespace filters
>   keys: Serialize ownership transfers with key accounting
> 
>  security/keys/key.c     | 15 +++++++++++++--
>  security/keys/keyctl.c  |  6 +++++-
>  security/keys/keyring.c |  7 ++++++-
>  security/keys/proc.c    | 15 +++++++++++++--
>  4 files changed, 37 insertions(+), 6 deletions(-)
> 
> 
> base-commit: 22430ae5d90ab288b0ee2ad99ae941f4a666b694
> -- 
> 2.43.0
> 

Hi overall this looks good (or better). I'm off work but I'll
look these up first thing in the morning and likely pick them
too.

Thanks for the patience and hope you understand that my previous
feedback was meant to be constructive :-)

Br, Jarkko