Hi, On Fri, Jul 01, 2016 at 08:00:03PM +0300, Tanu Kaskinen wrote: > On Fri, 2016-07-01 at 10:22 +0200, David Henningsson wrote: > > On 2016-07-01 03:30, Ahmed S. Darwish wrote: > > > On Fri, Jun 24, 2016 at 05:21:53PM +0200, David Henningsson wrote: > > > > Suppose the following chain of events: > > > > > > > > 1. Protocol-native loads > > > > 2. Access control module loads, puts hooks into existing protocol-native > > > > instance > > > > 3. Protocol-native unloads > > > > 4. Another Protocol-native loads > > > > > > > > At this point the new protocol-native is unprotected, which is probably not > > > > what you want? > > > > > > > > Also we have to make sure we don't get use-after-free errors, no matter > > > > which order the modules are unloaded. But that's certainly solvable, just an > > > > additional thing to remember. > > > > > > Can we solve the above two points by by introducing a pulse module > > > dependencies feature, just like what the linux kernel does? [1] > > > > > > If so, we can do the following: > > > > > > (a) Introduce module dependencies > > > > > > (b) Put the access-control hooks inside protocol-native itself, > > > and thus remove the 1-to-1 mappings [2] and avoid making core > > > dependent on protocol-native (even implicitly) > > > > > > (c) Make all policy modules, which contains all the hooks > > > implementations, hard-dependent on protocol-native > > > > > > In the first point raised, to unload protocol-native, user will > > > have to unload the dependent protocol-native-policy-X first. So if > > > the user loads protocol-native again, he/she will know that the > > > policy module will need to be reloaded as well .. > > > > > > In the second point raised, hopefully module dependencies will > > > simplify the relationship: use-after-free errors can be eliminated > > > given proper cleanup by the policy modules .. > > > > > > Any thoughts on this? > > > > > > > Just looking at core.h there seems to be both PA_CORE_HOOK_MODULE_NEW > > and PA_CORE_HOOK_MODULE_UNLINK hooks, perhaps it could be as simple as > > just requiring any access control module to listen to these and take the > > appropriate action? > > Yup, that's much easier indeed. > > As for whether or not a generic "module dependencies" feature would be > > good or bad: I would find it more user friendly myself if the user would > > not have to manually reload access control modules, but since I'm not > > that involved anymore, I don't think I should have decision power on > > that matter. :-) > > I agree that using MODULE_NEW and MODULE_UNLINK would be preferable to > a module dependency system if there actually was any danger of use- > after-free problems, but that issue doesn't exist. The access policy > module presumably holds a reference to the pa_native_protocol object, > which is a reference counted singleton, which will not be freed as long > as someone holds a reference. There's no 1:1 mapping to module-native- > protocol instances - there may be multiple instances loaded, but only > one pa_native_protocol object will ever exist. > Good point! I guess MODULE_NEW and MODULE_UNLINK can still have another good use in the policy module for fixing the earlier-mentioned case of load policy, unload protocol-native, then load it again, thus leaving the new protocol-native unsecured. Or will loading the new protocol-native be irrelevant since the older singleton object didn't get removed due to the reference count? will investigate .. In all cases, to keep everyone updated: we were disussing two proposed designs for implementing access-control in the latest weekly meeting. Option #1 represents a separation of concerns, proposed by Arun over IRC, and option #2 integrates access control with protocol-native as done in this patch series. [1] After some (amazingly quick!) working prototypes by Arun during the weekly, [2] it seems there are no major roadblocks to the first option. I've also had some initial code in the weekend, and the more code I've put into it, the more elegant this solution felt over design option #2. As a result of the above, we've agreed â?? in a quick discussion today's morning with Arun â?? to build access control using the first "separation of concerns" option. [1] In case I'll face a _major_ roadblock during the implementation of option #1, we will favor back option #2 where the hooks are coupled inside protocol native. At that point the issue of protocol-native being reference-counted, as shown by Tanu above, will be revisited. Thanks a lot everyone, [1] https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/AccessControl/#proposeddesigns [2] https://cgit.freedesktop.org/~arun/pulseaudio/log/ -- Darwish http://darwish.chasingpointers.com
