On 12/12/18 7:40 AM, Michal Privoznik wrote:
Similarly to what I did in DAC driver, this also requires the
same SELinux label to be used for shared paths. If a path is
already in use by a domain (or domains) then and the domain we
are starting now wants to access the path it has to have the same
SELinux label. This might look too restrictive as the new label
can still guarantee access to already running domains but in
reality it is very unlikely and usually an admin mistake.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/security/security_selinux.c | 177 +++++++++++++++++++++++---------
1 file changed, 130 insertions(+), 47 deletions(-)
[...]
static int
@@ -1362,7 +1429,8 @@ getContext(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
* errors that the caller(s) are already dealing with */
static int
virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
- const char *path)
+ const char *path,
+ bool recall)
{
bool privileged = virSecurityManagerGetPrivileged(mgr);
struct stat buf;
@@ -1386,26 +1454,35 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
goto cleanup;
}
- if (stat(newpath, &buf) != 0) {
- VIR_WARN("cannot stat %s: %s", newpath,
- virStrerror(errno, ebuf, sizeof(ebuf)));
- goto cleanup;
- }
-
- if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
- /* Any user created path likely does not have a default label,
- * which makes this an expected non error
- */
- VIR_WARN("cannot lookup default selinux label for %s", newpath);
- ret = 0;
- goto cleanup;
- }
-
- if ((rc = virSecuritySELinuxTransactionAppend(path, fcon, false, true)) < 0)
+ if ((rc = virSecuritySELinuxTransactionAppend(path, NULL, false, true)) < 0)
return -1;
else if (rc > 0)
return 0;
Since you've touched the code, Coverity looks again and determines that
@newpath can be leaked above
John
+ if (recall) {
+ if ((rc = virSecuritySELinuxRecallLabel(newpath, &fcon)) < 0) {
+ goto cleanup;
+ } else if (rc > 0) {
+ ret = 0;
+ goto cleanup;
+ }
+ } else {
+ if (stat(newpath, &buf) != 0) {
+ VIR_WARN("cannot stat %s: %s", newpath,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ goto cleanup;
+ }
+
+ if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) {
+ /* Any user created path likely does not have a default label,
+ * which makes this an expected non error
+ */
+ VIR_WARN("cannot lookup default selinux label for %s", newpath);
+ ret = 0;
+ goto cleanup;
+ }
+ }
+
[...]