On Tue, Jun 13, 2023 at 04:13:57PM +0200, Takashi Iwai wrote:
On Tue, 13 Jun 2023 16:00:34 +0200,
Oswald Buddenhagen wrote:
On Tue, Jun 13, 2023 at 01:08:55PM +0200, Takashi Iwai wrote:
> Hmm I don't get it; if an application just toggles the kctl value
> between two values in an infinite loop, it'll delete and recreate
> kctls endlessly as well with your patch, no?
>
yeah, but why should it toggle just so? it's not reasonable to do
that.
I'm arguing about a malicious or buggy applications. Don't ask logics
or conscience behind it.
yes, that was exactly the point of the sentence you cut away. it can be
broken in any number of "creative" ways. there is absolutely no point in
trying to prevent that.
the notion of "malicious" is meaningless in this context. a valid attack
vector would allow the application to do something that i cannot do
otherwise. hogging a cpu thread while flooding the system with
meaningless ioctls is something an app can do regardless, so whatever.
>> also, i don't think that disabling would be fundamentally different
>> from deleting: the particular code paths taken are somewhat different,
>> but the high-level view is essentially the same. so we can't really
>> make predictions which one would work better.
>
> Creating and deleting needs a lot of different works and much heavier
> tasks.
>
it's entirely plausible that an application would tear down structures
in response to controls being disabled, too.
But it's less dangerous.
if the app does mostly the same in both cases, then obviously neither
one is any less dangerous than the other one.
there is also the opposite angle to this, which makes it an own goal for
you: if the app did in fact respond to the elements being disabled by
merely disabling them in the user interface, then having the currently
inactive (but superficially identical) controls at all times would
contribute to a rather horrible user experience. so for this reason
alone it's better to actually delete the inapplicable set of controls.
> And, above all, many user-space programs will be borked if an
> element goes away, simply crashing. Some (rather rare) nice ones will
> still survive, though. I've learned this from the past.
>
yeah, but why should we care? it's not a regression when something new
doesn't work with some crappy pre-existing code.
We can't break user-space. That's a rule set in stone.
that rule means that we may not cause regressions, which we would not.
Well, then another, maybe foremost reason: you can't create / delete
kctls from the callback, simply because the callbacks are called in
the read lock. Adding / deleting an element may crash the another
concurrent task that traverses the list.
that would indeed be a problem, but fortunately the put() callback is
nowadays invoked with a write lock (see also commit 06405d8ee).
also, please go back to the first paragraph of the commit message of
patch 5 in the series.
regards,
ossi
