On 18.07.2013 01:08, Eric Blake wrote:
> Commit 75c1256 states that virGetGroupList must not be called
> between fork and exec, then commit ee777e99 promptly violated
> that for lxc's use of virSecurityManagerSetProcessLabel. Hoist
> the supplemental group detection to the time that the security
> manager is created. Qemu is safe, as it uses
> virSecurityManagerSetChildProcessLabel which in turn uses
> virCommand to determine supplemental groups.
>
> This does not fix the fact that virSecurityManagerSetProcessLabel
> calls virSecurityDACParseIds calls parseIds which eventually
> calls getpwnam_r, which also violates fork/exec async-signal-safe
> safety rules, but so far no one has complained of hitting
> deadlock in that case.
>
> * src/security/security_dac.c (_virSecurityDACData): Track groups
> in private data.
> (virSecurityDACPreFork): New function, to set them.
> (virSecurityDACClose): Clean up new fields.
> (virSecurityDACGetIds): Alter signature.
> (virSecurityDACSetSecurityHostdevLabelHelper)
> (virSecurityDACSetChardevLabel, virSecurityDACSetProcessLabel)
> (virSecurityDACSetChildProcessLabel): Update callers.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/security/security_dac.c | 63 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 19 deletions(-)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 9b5eaa8..00d04bf 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -43,6 +43,8 @@ typedef virSecurityDACData *virSecurityDACDataPtr;
> struct _virSecurityDACData {
> uid_t user;
> gid_t group;
> + gid_t *groups;
> + int ngroups;
> bool dynamicOwnership;
> };
>
> @@ -146,7 +148,8 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
>
> static int
> virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
> - uid_t *uidPtr, gid_t *gidPtr)
> + uid_t *uidPtr, gid_t *gidPtr,
> + gid_t **groups, int *ngroups)
> {
> int ret;
>
> @@ -157,8 +160,13 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
> return -1;
> }
>
> - if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0)
> + if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) {
> + if (groups)
> + *groups = NULL;
> + if (ngroups)
> + ngroups = 0;
I believe you wanted *ngroups = 0; in here.
> return ret;
> + }
>
> if (!priv) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list