On Tue, Jun 13, 2023 at 05:43:58PM +0200, Takashi Iwai wrote:
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.
Adding/deleting kctl increases the numid. It grows and grows.
as the code handles numid wraparound just fine, that would be a rather
pointless attack.
Crashing an existing application is the worst-case scenario.
a new driver (which this effectively is) crashing a broken application
is perfectly legitimate, as it doesn't affect any existing users.
that would indeed be a problem, but fortunately the put() callback is
nowadays invoked with a write lock (see also commit 06405d8ee).
Oh well, that's really not a change to be advertised for creating /
deleting kctls from the put callback at all.
and? it's done, and it's basically impossible to revert. so we may reap
its full benefits just as well, as i did in that previous commit.
Sorry, but my answer is same: NO. I see no reason why kctl deletion
and creation _must_ be implemented _inevitably_ in that way.
being the most straight-forward way to implement it certainly qualifies
as a good reason for doing it that way.
and i still see no convincing reason why it shouldn't.
Actually, snd_ctl_remove() should be changed back to a version that
takes the lock by itself instead. There is no reason to have a helper
without the lock called from leaf drivers.
well, except that this driver shows that there _is_ a reason. one may
choose to throw stones in one's own way, but that's rarely a wise
decision ...
regards,
ossi
