Messages in this thread | | | Date | Thu, 14 Dec 2023 14:44:28 +0000 | From | Luís Henriques <> | Subject | Re: [RFC PATCH] keys: flush work when accessing /proc/key-users |
| |
Hi David,
On Mon, Dec 11, 2023 at 02:02:47PM +0000, David Howells wrote: <snip> > > However, that would only fix the flakiness of the key quota for fs/crypto/, > > not for other users of the keyrings service. Maybe this suggests that > > key_put() should release the key's quota right away if the key's refcount > > drops to 0? > > That I would be okay with as the key should be removed in short order. > > Note that you'd have to change the spinlocks on key->user->lock to irq-locking > types. Or maybe we can do without them, at least for key gc, and use atomic > counters. key_invalidate() should probably drop the quota also.
I was trying to help with this but, first, I don't think atomic counters would actually be a solution. For example, we have the following in key_alloc():
spin_lock(&user->lock); if (!(flags & KEY_ALLOC_QUOTA_OVERRUN)) { if (user->qnkeys + 1 > maxkeys || user->qnbytes + quotalen > maxbytes || user->qnbytes + quotalen < user->qnbytes) goto no_quota; } user->qnkeys++; user->qnbytes += quotalen; spin_unlock(&user->lock);
Thus, I don't think it's really possible to simply stop using a lock without making these checks+changes non-atomic.
As for using spin_lock_irq() or spin_lock_irqsave(), my understanding is that the only places where this could be necessary is in key_put() and, possibly, key_payload_reserve(). key_alloc() shouldn't need that.
Finally, why would key_invalidate() require handling quotas? I'm probably just missing some subtlety, but I don't see the user->usage refcount being decremented anywhere in that path (or anywhere else, really).
Cheers, -- Luís
| |