There is a bug in the selinux patch we ship in RHEL5. When changing permissions or selinux context, if we were unable to read the selinux context before, then we crash. This happens e.g. if you disable selinux and create a file without a selinux context, but its also been seen on filesystems without selinux support (like ntfs). This is the upstream bug: http://bugzilla.gnome.org/show_bug.cgi?id=362302 The fix is simple, just avoid strcmp() with NULL in this case. Propose this for RHEL5
Created attachment 146283 [details] New fixed version of selinux patch
This request was evaluated by Red Hat Product Management for inclusion in a Red Hat Enterprise Linux major release. Product Management has requested further review of this request by Red Hat Engineering, for potential inclusion in a Red Hat Enterprise Linux Major release. This request is not yet committed for inclusion.
To reproduce easily, run with selinux disabled, create a new file (this should make it not have a selinux context), then bring up the properties dialog for the file and try to change the selinux context.
QE ack for RHEL5. It's a regression and a very visible error.
Building fix as nautilus-2.16.2-6.el5
Alex, how much of that patch is new vs. changed. I assume the NULL check was added in: + if (file->details->info->selinux_context != NULL && + strcmp(selinux_context, file->details->info->selinux_context) == 0) { ...but I'm trying to remember if the matchpathcon() stuff was there before (I think not, and it's just similar to another usage of matchpathcon() for "reset to default"). I'm also not sure the patch is "right", it'll fix the crash and it'll "work" on ext3 ... but I'm not sure what happens on NFS/VFAT etc. when you can't change the context (as I think it'll want to change it a lot now). I'm not 100% on when nautilus will try to set that new context: only when something else changes, only when the context is changed to something else, or all the time? -- I think it's the later. Also AIUI the result of matchpathcon() is being used to display a context to the user, when no context is available. This isn't right, as the kernel will not be treating that file as if it has that context (it'll treating it as a *:file_t). This will only confuse the user. This almost seems like the kernel should be fixed to return the "default" type it's using for the file, instead of nothing. Of course getting that change through for RHEL-5 might be a bigger PITA than this change.
The file->details->info->selinux_context != NULL part is the only thing i added. I tried to run it with selinux turned off on ext3 and didn't see any problems. Also, setting the selinux context via gnome-vfs never fails if selinux is disabled, as it checks set_selinux_context() and if that fails always returns OK. So, i think the change is "safe". Whether it does the right thing wrt selinux and matchpathcon() etc. I don't know, i don't really know much about selinux.
No crashes here either.
Closing out with results from 20070126.0 trees.