Re: [libvirt] [PATCHv1.5 3/8] security: DAC: Remove superfluous link resolution
On 07/22/2014 05:20 AM, Peter Krempa wrote:
When restoring security labels in the dac driver the code would
resolve
the file path and use the resolved one to be chown-ed. The setting code
doesn't do that. Remove the unnecessary code.
---
src/security/security_dac.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
Code has been there since it was first introduced by commit id
'15f5eaa0'. I see the security_selinux.c does the same thing and it
predates security_dac.c... Digging deeper finds commit id 'c86afc85e'
which added searching for the path for some unknown/unlogged reason.
One can only wonder why it was felt the restore needed link resolution,
but yet not necessary on the set.
While the Set functions don't necessarily have it, looking at all
callers of virFileResolveLink() causes me to believe there was some
reason (or some path) that perhaps didn't store the Resolved link and
this code is just ensuring that.
Perhaps Eric or DanB could chime in on this as the restore is a fairly
generic path, while the set paths are usually fairly specific.
Not 100% comfortable with giving an ACK
John
diff --git a/src/security/security_dac.c
b/src/security/security_dac.c
index 4d2a9d6..cdb2735 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -264,27 +264,10 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
static int
virSecurityDACRestoreSecurityFileLabel(const char *path)
{
- struct stat buf;
- int rc = -1;
- char *newpath = NULL;
-
VIR_INFO("Restoring DAC user and group on '%s'", path);
- if (virFileResolveLink(path, &newpath) < 0) {
- virReportSystemError(errno,
- _("cannot resolve symlink %s"), path);
- goto err;
- }
-
- if (stat(newpath, &buf) != 0)
- goto err;
-
/* XXX record previous ownership */
- rc = virSecurityDACSetOwnership(newpath, 0, 0);
-
- err:
- VIR_FREE(newpath);
- return rc;
+ return virSecurityDACSetOwnership(path, 0, 0);
}