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@xxxxxxxxxx>
> ---
> 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;
> + }
> + }
> +
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
