[Rpm-maint] Plugin findings
Hi, There's really nothing like field-testing when it comes to finding issues... Now that I simply have to run with the selinux plugin enabled (otherwise I'd have a very broken system real fast), hacking on new plugins revealed a some fairly nasty issues in the plugin system (and caused a fair bit of head-scratching early in the morning :) that's gone merrily unnoticed so far: When there are more than one plugins present, an unsupported hook (or errors like not finding the hook) in one plugin will cause all the remaining plugins to be skipped on that hook. This is because RPMPLUGINS_SET_HOOK_FUNC() does 'return ' on several conditions, which doesn't go very well with for-loops... Another related thing is that RPMPLUGINS_SET_HOOK_FUNC() is executed several times per each file in the transaction, times number of plugins loaded. It didn't matter for the collection plugins as they are so different in nature, but now with several hooks per each file its just terribly wasteful if nothing else. So... I'm planning on doing some fairly major surgery on the whole thing. Just checking whether you have some work-in-progress in this area, IIRC you were planning to look into changing the plugin initialization (move it much earlier etc) and I dont want to clash with that work if you've already started it. FWIW the kind of thing I have in mind is make plugins into "objects" that hold their own data (like name, symbol handle etc) and hooks are function pointers in the struct that are initialized when the plugin is first loaded so we dont have to rediscover the hook functions on each and every round, etc. - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Plugin findings
Hi, > There's really nothing like field-testing when it comes to finding issues... Now that I simply have to run with the selinux plugin enabled (otherwise I'd have a very broken system real fast), hacking on new plugins revealed a some fairly nasty issues in the plugin system (and caused a fair bit of head-scratching >early in the morning :) that's gone merrily unnoticed so far: I have actually only tested and use the hooks with one plugin (msm), so I guess some issues could have slipped though, indeed :( > When there are more than one plugins present, an unsupported hook (or errors like not finding the hook) in one plugin will cause all the remaining plugins to be skipped on that hook. This is because >RPMPLUGINS_SET_HOOK_FUNC() does 'return ' on several conditions, which doesn't go very well with for-loops... Oh, yes, I see this... One way would be to change it to a normal function that would return the state, but won't break the loop. The question is how to differentiate between "hook returned RPMRC_FAILED" and "hook isn't supported", which are indeed very very different things. >Another related thing is that RPMPLUGINS_SET_HOOK_FUNC() is executed several times per each file in the transaction, times number of plugins loaded. It didn't matter for the collection plugins as they are so different in nature, but now with several hooks per each file its just terribly wasteful if nothing >else. Hm.. How do you plan to avoid this? If you have let's say a security (selinux or msm) plugin and a log plugin, or two different security plugins (future LSM stacking), each plugin would need to get a hook called. > So... I'm planning on doing some fairly major surgery on the whole thing. Just checking whether you have some work-in-progress in this area, IIRC you were planning to look into changing the plugin initialization (move it much earlier etc) and I dont want to clash with that work if you've already started it. I have put aside the initialization work so far, since we were concentrating on fsm hooks, so there is nothing to clash with for sure! > FWIW the kind of thing I have in mind is make plugins into "objects" >that hold their own data (like name, symbol handle etc) and hooks are function pointers in the struct that are initialized when the plugin is first loaded so we dont have to rediscover the hook functions on each and every round, etc. This sounds good, would make it easier indeed! Best Regards, Elena. -Original Message- From: Panu Matilainen [mailto:pmati...@laiskiainen.org] Sent: Thursday, March 28, 2013 12:32 PM To: Reshetova, Elena Cc: rpm-maint@lists.rpm.org Subject: Plugin findings Hi, There's really nothing like field-testing when it comes to finding issues... Now that I simply have to run with the selinux plugin enabled (otherwise I'd have a very broken system real fast), hacking on new plugins revealed a some fairly nasty issues in the plugin system (and caused a fair bit of head-scratching early in the morning :) that's gone merrily unnoticed so far: When there are more than one plugins present, an unsupported hook (or errors like not finding the hook) in one plugin will cause all the remaining plugins to be skipped on that hook. This is because RPMPLUGINS_SET_HOOK_FUNC() does 'return ' on several conditions, which doesn't go very well with for-loops... Another related thing is that RPMPLUGINS_SET_HOOK_FUNC() is executed several times per each file in the transaction, times number of plugins loaded. It didn't matter for the collection plugins as they are so different in nature, but now with several hooks per each file its just terribly wasteful if nothing else. So... I'm planning on doing some fairly major surgery on the whole thing. Just checking whether you have some work-in-progress in this area, IIRC you were planning to look into changing the plugin initialization (move it much earlier etc) and I dont want to clash with that work if you've already started it. FWIW the kind of thing I have in mind is make plugins into "objects" that hold their own data (like name, symbol handle etc) and hooks are function pointers in the struct that are initialized when the plugin is first loaded so we dont have to rediscover the hook functions on each and every round, etc. - Panu - smime.p7s Description: S/MIME cryptographic signature ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Plugin findings
On 03/28/2013 12:52 PM, Reshetova, Elena wrote: Hi, There's really nothing like field-testing when it comes to finding issues... Now that I simply have to run with the selinux plugin enabled (otherwise I'd have a very broken system real fast), hacking on new plugins revealed a some fairly nasty issues in the plugin system (and caused a fair bit of head-scratching >early in the morning :) that's gone merrily unnoticed so far: I have actually only tested and use the hooks with one plugin (msm), so I guess some issues could have slipped though, indeed :( Same here, only tested with one plugin at a time until today :) When there are more than one plugins present, an unsupported hook (or errors like not finding the hook) in one plugin will cause all the remaining plugins to be skipped on that hook. This is because RPMPLUGINS_SET_HOOK_FUNC() does 'return ' on several conditions, which doesn't go very well with for-loops... Oh, yes, I see this... One way would be to change it to a normal function that would return the state, but won't break the loop. The question is how to differentiate between "hook returned RPMRC_FAILED" and "hook isn't supported", which are indeed very very different things. I suppose it could return RPMRC_NOTFOUND for non-supported hooks, but changing it to a function might not be entirely straightforward as it plays macro tricks to get the symbol names etc. Another related thing is that RPMPLUGINS_SET_HOOK_FUNC() is executed several times per each file in the transaction, times number of plugins loaded. It didn't matter for the collection plugins as they are so different in nature, but now with several hooks per each file its just terribly wasteful if nothing >else. Hm.. How do you plan to avoid this? If you have let's say a security (selinux or msm) plugin and a log plugin, or two different security plugins (future LSM stacking), each plugin would need to get a hook called. Oh, we can't avoid calling a million hooks and that doesn't bother me, it's the wholly redundant work of locating the plugin by its name, whether it supports a given hook and discovering the actual symbol via dlsym() over and over and over again on every single hook-call that is just ugly :) So... I'm planning on doing some fairly major surgery on the whole thing. Just checking whether you have some work-in-progress in this area, IIRC you were planning to look into changing the plugin initialization (move it much earlier etc) and I dont want to clash with that work if you've already started it. I have put aside the initialization work so far, since we were concentrating on fsm hooks, so there is nothing to clash with for sure! Ok. I'll go ahead with it then. FWIW the kind of thing I have in mind is make plugins into "objects" that hold their own data (like name, symbol handle etc) and hooks are function pointers in the struct that are initialized when the plugin is first loaded so we dont have to rediscover the hook functions on each and every round, etc. This sounds good, would make it easier indeed! Yup, besides making some things easier and new things possible, it'll probably simplify the whole thing as well. Lets see how it goes :) - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] FSM hooks for rpm plugin
On 03/27/2013 02:34 PM, Reshetova, Elena wrote: > >>> After far too much pondering... I went ahead and added the prepare >>> hook >>> + some related bits and pieces. And actually ripped out SELinux >>> + support >>>from rpm core while at it, replaced by a simple SELinux plugin. Wohoo. >> >> Looks cool :) Hope it works ;) >The basics seem to be working fine :) >The plugin configuration mechanism probably wants a bit more thinking + work >though: for some things you'd want to be able to enable a plugin by merely >installing the relevant (sub)package. For example I would want the SELinux >plugin to get enabled whenever its present, without having to hunt where > >__transaction_plugins is defined and override it, which is annoying and >error-prone. >In other words, I think there should be a drop-in directory for the plugin >configuration where the plugin sub-packages can drop their default >configuration as separate files, including whether they should be enabled by >default or not. There was something else in this direction too ... but I >can't remember it >right now. Yes, this is what I was wondering also. In our case I have the msm plugin in a separate rpm binary and our build engineers would like to have an easy way to switch it on and off, when building the image. And the most easiest way for them is to just include/not include the rpm plugin package. So, I think having this configurable separately would for sure make them happy and as you said probably would be much more robust than defining this in macros.in file. >> I think I'll leave the commit-hooks to you though :) > > Ok, I think I will be able to send you a version for review today, but > I have got one question. I was under impression that we at some point > agreed to pass to hooks the whole stat structure as opposite for just > mode_t. This would allow plugins to make checks on things like > st_nlink and other useful info about the file. Do I remember this wrongly? >No, you are right, but I chickened out :) See >http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=354c00ba7558e2dd78dd6f5906d3ba3e4c41e74a >http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=3ae4e4087e48e0bbfbafe5c9948bdcbc5fe1c63e >The trouble starts with the special case of fsmMkdirs(): there's a struct >stat handy, but the directories only get created if the stat() fails, in >which case the struct stat contents are undefined. Sure, it'd be possible to >hack up a struct stat that resembles a directory for that case, but that rang >a "proceed with >caution" alarm bell in my head. If, or rather when, we need >to fake up stuff the semantics get fuzzy real quick. For example the st_nlink >thing: the adjusted count is currently actually only available in fsmCommit() >so different hooks would see different values for the same file. Etc. >I still actually think we'll eventually want to pass the whole stat struct >(or roughly equivalent amount of data in some other means) to plugins, being >able to sanely do so requires further hacking of the FSM. Should we do the stat passing then for fsmCommit hooks? I am attaching the current state of my fsmCommit hooks just to show the place where I was thinking to add them. I haven't checked the tabs and other stuff, so this is just to give idea. I was thinking that instead of passing mode_t to the both hooks (in this patch it still does that), the whole stat stuct can be passed and this would give at least these hooks enough info. What do you think? Or if is too bad/assymetric to have mode_t in other hooks and stat here? Best Regards, Elena 0001-Adding-FSM_COMMIT_PRE-and-FSM_COMMIT_POST-plugin-hoo.patch Description: Binary data smime.p7s Description: S/MIME cryptographic signature ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
