On 12/20/18 12:39 AM, John Ferlan wrote:
>
>
> 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
Ah, right. this should have been "goto cleanup" instead of "return -1"
and "{ret = 0; goto cleanup}" instead of "return 0".
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
