Re: [PATCH v3] keys: finalize persistent keyring timeout after link attempt

[Jarkko Sakkinen]

On Wed, Sep 02, 2026 at 08:04:30PM +0200, Karl Mehltretter wrote:
> On Wed, Sep 02, 2026 at 02:00:56AM +0100, Jarkko Sakkinen wrote:
> > > +		*persistent_ref = find_key_to_update(reg_ref, index_key);
> > > +		if (*persistent_ref)
> > > +			return 1;
> > 
> > I'd return -ENOENT here instead and not make return value tristate.
> > 
> 
> I don't think using an error to mean "found" is safe. keyring_alloc()
> can return -ENOENT through security_key_alloc(). A BPF LSM key_alloc
> hook can return any errno.
> 
> I checked this in QEMU with a BPF LSM hook returning -ENOENT.
> keyring_alloc(".persistent_register") returned -ENOENT and
> KEYCTL_GET_PERSISTENT failed with ENOENT.
> 
> So -ENOENT would be ambiguous. I also considered +EEXIST instead of 1,
> but that seems a bit too clever. I'd rather keep 0/1/<0.

Fair enough.

Let's tweak documentation then:

 * Return 0 if a keyring was created, 1 if an existing keyring was found, or a
 * negative error.  On a nonnegative return, persistent_ref holds a reference
 * to the keyring.

How about being here a bit less convoluted and replace the text with:

 * Returns a boolean value on success case, which tell whether a new
 * keyring was created or not.

This is imho quite well in the ballpark with detail/clarity.


> 
> Thanks,
> Karl

BR, Jarkko