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