security_dac: rework callback parameter passing

Currently, the DAC security driver passes callback data as

    void params[2];
    params[0] = mgr;
    params[1] = def;

Clean this up by defining a structure for passing the callback
data.  Moreover, there's no need to pass the whole virDomainDef
in the callback as the only thing needed in the callbacks is
virSecurityLabelDefPtr.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
This commit is contained in:
Jim Fehlig 2014-05-15 12:15:01 -06:00
parent 1d98e713d5
commit 3de7e4ec5e

View File

@ -53,6 +53,14 @@ struct _virSecurityDACData {
char *baselabel; char *baselabel;
}; };
typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData;
typedef virSecurityDACCallbackData *virSecurityDACCallbackDataPtr;
struct _virSecurityDACCallbackData {
virSecurityManagerPtr manager;
virSecurityLabelDefPtr secdef;
};
/* returns -1 on error, 0 on success */ /* returns -1 on error, 0 on success */
int int
virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
@ -82,19 +90,12 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
/* returns 1 if label isn't found, 0 on success, -1 on error */ /* returns 1 if label isn't found, 0 on success, -1 on error */
static int static int
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) virSecurityDACParseIds(virSecurityLabelDefPtr seclabel,
uid_t *uidPtr, gid_t *gidPtr)
{ {
virSecurityLabelDefPtr seclabel; if (!seclabel || !seclabel->label)
if (def == NULL)
return 1; return 1;
seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (seclabel == NULL || seclabel->label == NULL) {
VIR_DEBUG("DAC seclabel for domain '%s' wasn't found", def->name);
return 1;
}
if (virParseOwnershipIds(seclabel->label, uidPtr, gidPtr) < 0) if (virParseOwnershipIds(seclabel->label, uidPtr, gidPtr) < 0)
return -1; return -1;
@ -103,31 +104,24 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
static int static int
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, virSecurityDACGetIds(virSecurityLabelDefPtr seclabel,
virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr, uid_t *uidPtr, gid_t *gidPtr,
gid_t **groups, int *ngroups) gid_t **groups, int *ngroups)
{ {
int ret; int ret;
if (!def && !priv) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to determine default DAC seclabel "
"for an unknown object"));
return -1;
}
if (groups) if (groups)
*groups = priv ? priv->groups : NULL; *groups = priv ? priv->groups : NULL;
if (ngroups) if (ngroups)
*ngroups = priv ? priv->ngroups : 0; *ngroups = priv ? priv->ngroups : 0;
if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) if ((ret = virSecurityDACParseIds(seclabel, uidPtr, gidPtr)) <= 0)
return ret; return ret;
if (!priv) { if (!priv) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("DAC seclabel couldn't be determined " _("DAC seclabel couldn't be determined"));
"for domain '%s'"), def->name);
return -1; return -1;
} }
@ -141,20 +135,12 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
/* returns 1 if label isn't found, 0 on success, -1 on error */ /* returns 1 if label isn't found, 0 on success, -1 on error */
static int static int
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
virSecurityDACParseImageIds(virDomainDefPtr def, virSecurityDACParseImageIds(virSecurityLabelDefPtr seclabel,
uid_t *uidPtr, gid_t *gidPtr) uid_t *uidPtr, gid_t *gidPtr)
{ {
virSecurityLabelDefPtr seclabel; if (!seclabel || !seclabel->imagelabel)
if (def == NULL)
return 1; return 1;
seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (seclabel == NULL || seclabel->imagelabel == NULL) {
VIR_DEBUG("DAC imagelabel for domain '%s' wasn't found", def->name);
return 1;
}
if (virParseOwnershipIds(seclabel->imagelabel, uidPtr, gidPtr) < 0) if (virParseOwnershipIds(seclabel->imagelabel, uidPtr, gidPtr) < 0)
return -1; return -1;
@ -163,25 +149,18 @@ virSecurityDACParseImageIds(virDomainDefPtr def,
static int static int
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, virSecurityDACGetImageIds(virSecurityLabelDefPtr seclabel,
virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr) uid_t *uidPtr, gid_t *gidPtr)
{ {
int ret; int ret;
if (!def && !priv) { if ((ret = virSecurityDACParseImageIds(seclabel, uidPtr, gidPtr)) <= 0)
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to determine default DAC imagelabel "
"for an unknown object"));
return -1;
}
if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) <= 0)
return ret; return ret;
if (!priv) { if (!priv) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("DAC imagelabel couldn't be determined " _("DAC imagelabel couldn't be determined"));
"for domain '%s'"), def->name);
return -1; return -1;
} }
@ -315,14 +294,14 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
size_t depth ATTRIBUTE_UNUSED, size_t depth ATTRIBUTE_UNUSED,
void *opaque) void *opaque)
{ {
void **params = opaque; virSecurityDACCallbackDataPtr cbdata = opaque;
virSecurityManagerPtr mgr = params[0]; virSecurityManagerPtr mgr = cbdata->manager;
virDomainDefPtr def = params[1]; virSecurityLabelDefPtr secdef = cbdata->secdef;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
uid_t user; uid_t user;
gid_t group; gid_t group;
if (virSecurityDACGetImageIds(def, priv, &user, &group)) if (virSecurityDACGetImageIds(secdef, priv, &user, &group))
return -1; return -1;
return virSecurityDACSetOwnership(path, user, group); return virSecurityDACSetOwnership(path, user, group);
@ -335,8 +314,9 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr,
virDomainDiskDefPtr disk) virDomainDiskDefPtr disk)
{ {
void *params[2];
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityDACCallbackData cbdata;
virSecurityLabelDefPtr secdef;
if (!priv->dynamicOwnership) if (!priv->dynamicOwnership)
return 0; return 0;
@ -344,12 +324,14 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr,
if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK)
return 0; return 0;
params[0] = mgr; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
params[1] = def;
cbdata.manager = mgr;
cbdata.secdef = secdef;
return virDomainDiskDefForeachPath(disk, return virDomainDiskDefForeachPath(disk,
false, false,
virSecurityDACSetSecurityFileLabel, virSecurityDACSetSecurityFileLabel,
params); &cbdata);
} }
@ -415,14 +397,14 @@ static int
virSecurityDACSetSecurityHostdevLabelHelper(const char *file, virSecurityDACSetSecurityHostdevLabelHelper(const char *file,
void *opaque) void *opaque)
{ {
void **params = opaque; virSecurityDACCallbackDataPtr cbdata = opaque;
virSecurityManagerPtr mgr = params[0]; virSecurityManagerPtr mgr = cbdata->manager;
virDomainDefPtr def = params[1]; virSecurityLabelDefPtr secdef = cbdata->secdef;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
uid_t user; uid_t user;
gid_t group; gid_t group;
if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL))
return -1; return -1;
return virSecurityDACSetOwnership(file, user, group); return virSecurityDACSetOwnership(file, user, group);
@ -462,8 +444,8 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
virDomainHostdevDefPtr dev, virDomainHostdevDefPtr dev,
const char *vroot) const char *vroot)
{ {
void *params[] = {mgr, def};
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityDACCallbackData cbdata;
int ret = -1; int ret = -1;
if (!priv->dynamicOwnership) if (!priv->dynamicOwnership)
@ -472,6 +454,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
return 0; return 0;
cbdata.manager = mgr;
cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
switch ((enum virDomainHostdevSubsysType) dev->source.subsys.type) { switch ((enum virDomainHostdevSubsysType) dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb; virUSBDevicePtr usb;
@ -485,8 +470,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (!usb) if (!usb)
goto done; goto done;
ret = virUSBDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, ret = virUSBDeviceFileIterate(usb,
params); virSecurityDACSetSecurityUSBLabel,
&cbdata);
virUSBDeviceFree(usb); virUSBDeviceFree(usb);
break; break;
} }
@ -509,11 +495,12 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
virPCIDeviceFree(pci); virPCIDeviceFree(pci);
goto done; goto done;
} }
ret = virSecurityDACSetSecurityPCILabel(pci, vfioGroupDev, params); ret = virSecurityDACSetSecurityPCILabel(pci, vfioGroupDev, &cbdata);
VIR_FREE(vfioGroupDev); VIR_FREE(vfioGroupDev);
} else { } else {
ret = virPCIDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, ret = virPCIDeviceFileIterate(pci,
params); virSecurityDACSetSecurityPCILabel,
&cbdata);
} }
virPCIDeviceFree(pci); virPCIDeviceFree(pci);
@ -533,8 +520,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (!scsi) if (!scsi)
goto done; goto done;
ret = virSCSIDeviceFileIterate(scsi, virSecurityDACSetSecuritySCSILabel, ret = virSCSIDeviceFileIterate(scsi,
params); virSecurityDACSetSecuritySCSILabel,
&cbdata);
virSCSIDeviceFree(scsi); virSCSIDeviceFree(scsi);
break; break;
@ -675,12 +663,15 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
{ {
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr seclabel;
char *in = NULL, *out = NULL; char *in = NULL, *out = NULL;
int ret = -1; int ret = -1;
uid_t user; uid_t user;
gid_t group; gid_t group;
if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL))
return -1; return -1;
switch ((enum virDomainChrType) dev->type) { switch ((enum virDomainChrType) dev->type) {
@ -902,6 +893,7 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
const char *stdin_path ATTRIBUTE_UNUSED) const char *stdin_path ATTRIBUTE_UNUSED)
{ {
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
size_t i; size_t i;
uid_t user; uid_t user;
gid_t group; gid_t group;
@ -909,6 +901,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
if (!priv->dynamicOwnership) if (!priv->dynamicOwnership)
return 0; return 0;
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
for (i = 0; i < def->ndisks; i++) { for (i = 0; i < def->ndisks; i++) {
/* XXX fixme - we need to recursively label the entire tree :-( */ /* XXX fixme - we need to recursively label the entire tree :-( */
if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR) if (virDomainDiskGetType(def->disks[i]) == VIR_STORAGE_TYPE_DIR)
@ -939,7 +933,7 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
return -1; return -1;
} }
if (virSecurityDACGetImageIds(def, priv, &user, &group)) if (virSecurityDACGetImageIds(secdef, priv, &user, &group))
return -1; return -1;
if (def->os.kernel && if (def->os.kernel &&
@ -963,11 +957,14 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def, virDomainDefPtr def,
const char *savefile) const char *savefile)
{ {
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
uid_t user; uid_t user;
gid_t group; gid_t group;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
if (virSecurityDACGetImageIds(def, priv, &user, &group)) secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (virSecurityDACGetImageIds(secdef, priv, &user, &group) < 0)
return -1; return -1;
return virSecurityDACSetOwnership(savefile, user, group); return virSecurityDACSetOwnership(savefile, user, group);
@ -992,13 +989,16 @@ static int
virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def) virDomainDefPtr def)
{ {
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
uid_t user; uid_t user;
gid_t group; gid_t group;
gid_t *groups; gid_t *groups;
int ngroups; int ngroups;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
if (virSecurityDACGetIds(def, priv, &user, &group, &groups, &ngroups)) secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (virSecurityDACGetIds(secdef, priv, &user, &group, &groups, &ngroups) < 0)
return -1; return -1;
VIR_DEBUG("Dropping privileges of DEF to %u:%u, %d supplemental groups", VIR_DEBUG("Dropping privileges of DEF to %u:%u, %d supplemental groups",
@ -1016,11 +1016,14 @@ virSecurityDACSetChildProcessLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def, virDomainDefPtr def,
virCommandPtr cmd) virCommandPtr cmd)
{ {
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
uid_t user; uid_t user;
gid_t group; gid_t group;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL))
return -1; return -1;
VIR_DEBUG("Setting child to drop privileges of DEF to %u:%u", VIR_DEBUG("Setting child to drop privileges of DEF to %u:%u",