Re: [Rpm-maint] Getting make check to work
On Wed, 2013-03-06 at 15:28 +0200, Panu Matilainen wrote: ./configure CPPFLAGS=`pkg-config --cflags nss` --with-external-db make make check [...] And finally, thanks for complaining about this, really. One can only explain known bugs so many times before it gets so irritating as to mandate a fix, which was the case here :) Thank you! All 266 tests were successful. Awesome, Mark ___ 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/06/2013 05:01 PM, Reshetova, Elena wrote: What's missing is that another call to the hook is needed in fsmMkdirs() for the unowned directories, and there we should perhaps pass in the unowned aspect somehow. Having a separate argument for that seems like an overkill though... maybe we could pass that piece of info in the action argument instead. One possibility could be adding a new action, eg FA_CREATEUNOWNED that could be used for unowned directories (and files, but there aren't any currently). Or we could define the action as a partial bitfield: leave the existing actions alone but reserve the upper byte for special bits, such as unknown. I had some other use-case for turning it into (partial) bitfield but can't remember what it was right now. I think bitfield would be better in this case: if one start introduce actions FA_CREATEUNOWNED, then why not have FA_CREATELINK and etc. I will add hook calling for mkdirs, but I think it is better that you do a change of actions in a separate commit. Sure, and I agree bitfield seems like the better option as it'll allow cramming a whole lot more information in there. There's a whole lot of redundancy in the current actions (all the skip cases for example) and some of the values are totally unused as well. As the file actions aren't really exported in the API in a way that somebody could actually be using them, we might be able to get away with just redefining the whole thing as a bitfield and add the old symbols as defines or'ed together from the bits. But I'll need to think about it some more, in any case such a total conversion is not required in order to add a handful of bits right now. Ok, I think if you can add an unowned bit to it for a start, this can be a good beginning and then even the existing pre/post hooks can get one less argument, which is great. Continue on bit field idea, it would be great for plugins to get the basic info about the file acted upon from action also: so , if we are adding unowned bit, should the basic bits indicating hard or sym link be also supplied in action? Like action could point that it is creation of symlink or removal of hard link. Is it too bad idea? Hmm, but we already pass the mode (and planning to pass the whole stat struct) around, so you can tell whether its a symlink, directory or something else. Hardlinks are the more special case but you can tell them apart from others by st_nlinks from the stat struct. Except that you dont necessarily know which one is real file without extra tracking... unless st_nlinks is 1 for real files (including the first hardlink) and more for the actual links. I'm not sure if that's the case even already, but should be possible to arrange at least. I am quite sure you can't tell it from stat struct which is real file and which is hard link: the value for st_nlinks is stored in inode and not in dentry in my understanding, that's why it isn't really easy for plugin to detect the hard links, so indicator would be a plus... Best Regards, Elena. 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] FSM hooks for rpm plugin
On 03/07/2013 03:59 PM, Reshetova, Elena wrote:
On 03/06/2013 05:01 PM, Reshetova, Elena wrote:
What's missing is that another call to the hook is needed in
fsmMkdirs() for the unowned directories, and there we should perhaps pass
in the unowned
aspect somehow. Having a separate argument for that seems like an
overkill though... maybe we could pass that piece of info in the
action argument instead. One possibility could be adding a new
action, eg FA_CREATEUNOWNED that could be used for unowned
directories (and files, but there aren't any currently). Or we could
define the action as a partial bitfield: leave the existing actions
alone but reserve the upper byte for special bits, such as
unknown. I had some other use-case for turning it into (partial) bitfield
but can't remember what it was right now.
I think bitfield would be better in this case: if one start introduce
actions FA_CREATEUNOWNED, then why not have FA_CREATELINK and etc. I
will add hook calling for mkdirs, but I think it is better that you do
a change of actions in a separate commit.
Sure, and I agree bitfield seems like the better option as it'll allow
cramming a whole lot more information in there. There's a whole lot of
redundancy in the current actions (all the skip cases for example) and some
of the values are totally unused as well.
As the file actions aren't really exported in the API in a way that somebody
could actually be using them, we might be able to get away with just
redefining the whole thing as a bitfield and add the old symbols as defines
or'ed together from the bits. But I'll need to think about it some more, in
any case such a total conversion is not required in order to add a handful
of bits right now.
Ok, I think if you can add an unowned bit to it for a start, this can be a
good beginning and then even the existing pre/post hooks can get one less
argument, which is great.
Sure, will do.
Continue on bit field idea, it would be great for plugins to get the
basic info about the file acted upon from action also: so , if we are
adding unowned bit, should the basic bits indicating hard or sym link
be also supplied in action? Like action could point that it is
creation of symlink or removal of hard link. Is it too bad idea?
Hmm, but we already pass the mode (and planning to pass the whole stat
struct) around, so you can tell whether its a symlink, directory or something
else. Hardlinks are the more special case but you can tell them apart from
others by st_nlinks from the stat struct. Except that you dont necessarily
know which one is real file without extra tracking... unless st_nlinks is 1
for real files (including the first
hardlink) and more for the actual links. I'm not sure if that's the case even
already, but should be possible to arrange at least.
I am quite sure you can't tell it from stat struct which is real file and
which is hard link: the value for st_nlinks is stored in inode and not in
dentry in my understanding, that's why it isn't really easy for plugin to
detect the hard links, so indicator would be a plus...
A file is a hard-link if (S_ISREG(st-st_mode) st-st_nlink 1) is
true. When erasing, we get this info from filesystem so that remains
accurate (the last one would be seen as the real file). On
installation the stat struct of a file is made up by rpm, so we can pass
whatever we want in there. Currently st_nlink for hardlinks equals the
total number of links a file will have, but we can easily change that to
the number of *current* links so that it better matches reality. Ie the
first one will have st_nlink == 1 so it will be seen as the real file,
the rest will st_nlink++ each. See attached patch for a quick
implementation of this.
- Panu -
diff --git a/lib/fsm.c b/lib/fsm.c
index f28010c..5bd5bfa 100644
--- a/lib/fsm.c
+++ b/lib/fsm.c
@@ -1052,7 +1052,7 @@ static int fsmMakeLinks(FSM_t fsm, hardLink_t li)
return ec;
}
-static int fsmCommit(FSM_t fsm, int ix, int setmeta);
+static int fsmCommit(FSM_t fsm, int ix, const struct stat * st);
/** \ingroup payload
* Commit hard linked file set atomically.
@@ -1065,8 +1065,7 @@ static int fsmCommitLinks(FSM_t fsm)
const char * nsuffix = fsm-nsuffix;
struct stat * st = fsm-sb;
int rc = 0;
-int setmeta = 1;
-nlink_t i;
+nlink_t i, link_count = 1;
hardLink_t li;
fsm-path = NULL;
@@ -1081,10 +1080,10 @@ static int fsmCommitLinks(FSM_t fsm)
if (li-filex[i] 0) continue;
rc = fsmMapPath(fsm, li-filex[i]);
if (!XFA_SKIPPING(fsm-action)) {
- rc = fsmCommit(fsm, li-filex[i], setmeta);
- /* only the first created link needs permissions etc to be set */
+ st-st_nlink = link_count;
+ rc = fsmCommit(fsm, li-filex[i], st);
if (!rc)
- setmeta = 0;
+ link_count++;
}
fsm-path = _free(fsm-path);
li-filex[i] = -1;
@@ -1542,10 +1541,9 @@ static int fsmBackup(FSM_t fsm)
return rc;
}
-static int fsmCommit(FSM_t fsm, int ix, int setmeta)
Re: [Rpm-maint] FSM hooks for rpm plugin
On 03/07/2013 04:27 PM, Reshetova, Elena wrote: If we now see a proper use case: like pre-commit can be used for content checking or even checksum calculation (I would still probably prefer to calculate it when it is read from archive: feels a bit safer that way, while I understand logically that if file is in tmp location and properly protected, nothing should happen to it anyway). Yup, if you get it directly from the source then nothing can be tampering with it, and certainly for at least checksum calculation is going to be more efficient if you can do it on the fly (but then rpm can do multiple checksums already, what's missing is exporting that functionality in a sane way). OTOH if something can tamper with the file that we're creating then it can do nasty stuff anyway, so its perhaps a bit academic. Yes, I agree, I guess I went too paranoid on this, which I guess happens if you work related to security :) Yeah, occupational hazards. Each job has their own :) Another concrete use-case could be a plugin to put %config files into a version control of some kind. While this too would mostly be interested in actually created/erased files, it might actually want to know about %ghosts and such to be able to snapshot changes made by admin that might've occurred outside of the package. Guess this needs further thought, or better yet, actually trying to do it, but this might be a use-case for the current hooks as they are now, getting called for all files of a package whether actually touched or not. This almost drives us to have this set of hooks: 1) pre/post where they are now 2) metadata hook (both install and remove) 3) pre fsm commit hook for checking on a file content () Indeed needs more thinking Naah, we just need more hooks! :P Until we get so many of them that we can't manage them anymore and they are going to live on their own ;) Seriously though, I've started thinking of that very same set of hooks so we're probably not too far off-target here. But now I find myself wondering whether 2) is actually necessary afterall: if we have pre- (and perhaps post too) commit hook, and can pass in some additional information as file action flag bits... we could have something like FA_SETMETA bit for the entries where its appropriate (ie non-hardlinks I guess), and if there's a post-commit hook as well, the plugins can decide whether to set permissions before or after rpm. If instead of hook 2) we have a post fsm commit hook that with proper parameter, I think it can be serving the same purpose. The only thing is that setting permissions before rpm does it isn't really possible in this case, because before fsmcommit() file isn't in final location yet. But again, I guess rpm would always trust its own plugins by a nature of a system setup, so we can assume that plugins won't try to change permissions that would break rpm query or they would break their own system in that case. Yup, rpm pretty much has to trust its plugins. OTOH... this made me think of another related issue: it would actually be better to set the permissions etc before moving the file to its final location. Currently we first move the file and then start setting permissions, which means executables and all will for a short period of time have totally incorrect permissions, labels and all. So if you happen to execute that binary during that period, who knows what will happen: it could fail to execute at all, execute with wrong capabilities / labels etc. Setting the permissions before moving would fix that and also avoid replacing a previous file at all in case we fail to in one of the metadata steps. For the stock metadata the actual path makes no difference, but for security labels you'd want the final path though (to avoid having to figure out and strip the temp extension from the file), so it'd require passing two paths to the pre-commit hook: current and final. - 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/07/2013 07:55 PM, Panu Matilainen wrote: On 03/07/2013 04:27 PM, Reshetova, Elena wrote: If instead of hook 2) we have a post fsm commit hook that with proper parameter, I think it can be serving the same purpose. The only thing is that setting permissions before rpm does it isn't really possible in this case, because before fsmcommit() file isn't in final location yet. But again, I guess rpm would always trust its own plugins by a nature of a system setup, so we can assume that plugins won't try to change permissions that would break rpm query or they would break their own system in that case. Yup, rpm pretty much has to trust its plugins. OTOH... this made me think of another related issue: it would actually be better to set the permissions etc before moving the file to its final location. Currently we first move the file and then start setting permissions, which means executables and all will for a short period of time have totally incorrect permissions, labels and all. So if you happen to execute that binary during that period, who knows what will happen: it could fail to execute at all, execute with wrong capabilities / labels etc. Setting the permissions before moving would fix that and also avoid replacing a previous file at all in case we fail to in one of the metadata steps. For the stock metadata the actual path makes no difference, but for security labels you'd want the final path though (to avoid having to figure out and strip the temp extension from the file), so it'd require passing two paths to the pre-commit hook: current and final. ...or we could open an fd to the temporary file and pass that along with the final pathname. At least all the standard metadata + file capabilities + selinux labels are settable via file descriptor as well as by path so for those the fd approach wouldn't be a problem. - 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/07/2013 08:05 PM, Panu Matilainen wrote: On 03/07/2013 07:55 PM, Panu Matilainen wrote: On 03/07/2013 04:27 PM, Reshetova, Elena wrote: If instead of hook 2) we have a post fsm commit hook that with proper parameter, I think it can be serving the same purpose. The only thing is that setting permissions before rpm does it isn't really possible in this case, because before fsmcommit() file isn't in final location yet. But again, I guess rpm would always trust its own plugins by a nature of a system setup, so we can assume that plugins won't try to change permissions that would break rpm query or they would break their own system in that case. Yup, rpm pretty much has to trust its plugins. OTOH... this made me think of another related issue: it would actually be better to set the permissions etc before moving the file to its final location. Currently we first move the file and then start setting permissions, which means executables and all will for a short period of time have totally incorrect permissions, labels and all. So if you happen to execute that binary during that period, who knows what will happen: it could fail to execute at all, execute with wrong capabilities / labels etc. Setting the permissions before moving would fix that and also avoid replacing a previous file at all in case we fail to in one of the metadata steps. For the stock metadata the actual path makes no difference, but for security labels you'd want the final path though (to avoid having to figure out and strip the temp extension from the file), so it'd require passing two paths to the pre-commit hook: current and final. ...or we could open an fd to the temporary file and pass that along with the final pathname. At least all the standard metadata + file capabilities + selinux labels are settable via file descriptor as well as by path so for those the fd approach wouldn't be a problem. ...except that doesn't work for symlinks. So scratch that idea. We need to construct both paths in commit anyway so there's no added cost or trouble, just needs changing fsmCommit() a little bit. If the number of arguments starts growing too long, one option could be stuffing them into a struct whose pointer we hand to the hooks. Actually, taking that a little bit further, we could hand an opaque file info handle to the hooks and force the hooks to go through an API to get the data, which would allow changing implementation details and adding more data without affecting compatibility. And yes I'm getting ahead of the schedule here, just thinking out loud :) - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
