Re: [RFC] struct filename, io_uring and audit troubles

[Jens Axboe <axboe@kernel.dk>]

On 9/23/24 9:07 AM, Al Viro wrote:
> On Mon, Sep 23, 2024 at 12:30:48AM -0600, Jens Axboe wrote:
> 
>> 1) Just don't reuse the entry. Then we can drop the struct
>>    filename->aname completely as well. Yes that might incur an extra
>>    alloc for the odd case of audit_enabled and being deep enough that
>>    the preallocated names have been used, but doesn't anyone really
>>    care? It'll be noise in the overhead anyway. Side note - that would
>>    unalign struct filename again. Would be nice to drop audit_names from
>>    a core fs struct...
> 
> You'll get different output in logs, though.  Whether that breaks userland
> setups/invalidates certifications/etc.... fuck knows.

No idea about that... But I'd say without strong evidence that this
breaks userland for something as odd as audit, well... And honestly
really a layering problem that struct filename has an audit link in
there.

> If anything, a loop through the list, searching for matching entry would
> be safer in that respect.  Order of the items... might or might not be
> an issue - see above.
> 
>> 2) Add a ref to struct audit_names, RCU kfree it when it drops to zero.
>>    This would mean dropping struct audit_context->preallocated_names, as
> 
> Costly, that.

For sure. And you could keep preallocated_names if you rcu free the
context too. But I strongly believe that approach #1 is, by far, the
cheaper alternative. If we can tolerate the ordering potentially
changing.

>>    otherwise we'd run into trouble there if a context gets blown away
>>    while someone else has a ref to that audit_names struct. We could do
>>    this without a ref as well, as long as we can store an audit_context
>>    pointer in struct audit_names and be able to validate it under RCU.
>>    If ctx doesn't match, don't use it.
> 
> That's one of the variants I mentioned upthread...

Sorry, still away on travels and conferences, so haven't been keeping up
on replies.

-- 
Jens Axboe