linux-kernel - Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jul 2017 11:40:55 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Claudio Imbrenda <imbrenda@...ux.vnet.ibm.com>, kvm@...r.kernel.org
Cc:     borntraeger@...ibm.com, pbonzini@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a
 VM

On 10.07.2017 11:20, Claudio Imbrenda wrote:

Minor minor nit:

The subject should state "creating or destroying a VM"

> This patch adds a few lines to the KVM common code to fire a
> KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
> carries five environment variables:
> 
> CREATED indicates how many times a new VM has been created. It is
> 	useful for example to trigger specific actions when the first
> 	VM is started
> COUNT indicates how many VMs are currently active. This can be used for
> 	logging or monitoring purposes
> PID has the pid of the KVM process that has been started or stopped.
> 	This can be used to perform process-specific tuning.
> STATS_PATH contains the path in debugfs to the directory with all the
> 	runtime statistics for this VM. This is useful for performance
> 	monitoring and profiling.
> EVENT described the type of event, its value can be either "create" or
> 	"destroy"
> 
> Specific udev rules can be then set up in userspace to deal with the
> creation or destruction of VMs as needed.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@...ux.vnet.ibm.com>
> ---
>  virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f0fe9d0..7c39783 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -130,6 +130,12 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
>  
>  static bool largepages_enabled = true;
>  
> +#define KVM_EVENT_CREATE_VM 0
> +#define KVM_EVENT_DESTROY_VM 1
> +static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm);
> +static unsigned long long kvm_createvm_count;
> +static unsigned long long kvm_active_vms;
> +
>  bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
>  {
>  	if (pfn_valid(pfn))
> @@ -728,6 +734,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  	int i;
>  	struct mm_struct *mm = kvm->mm;
>  
> +	kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
>  	kvm_destroy_vm_debugfs(kvm);
>  	kvm_arch_sync_events(kvm);
>  	spin_lock(&kvm_lock);
> @@ -3196,6 +3203,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
>  		fput(file);
>  		return -ENOMEM;
>  	}
> +	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
>  
>  	fd_install(r, file);
>  	return r;
> @@ -3848,6 +3856,57 @@ static const struct file_operations *stat_fops[] = {
>  	[KVM_STAT_VM]   = &vm_stat_fops,
>  };
>  
> +static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
> +{
> +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];

do we really need that much space for a pid?

> +	const char pathvar[11] = "STATS_PATH=";
> +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
> +	char *tmp, *pathbuf;
> +	unsigned long long created, active;
> +	int idx = 3;
> +
> +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
> +		return;
> +
> +	spin_lock(&kvm_lock);
> +	if (type == KVM_EVENT_CREATE_VM) {
> +		kvm_createvm_count++;
> +		kvm_active_vms++;
> +	} else if (type == KVM_EVENT_DESTROY_VM) {
> +		kvm_active_vms--;
> +	}
> +	created = kvm_createvm_count;
> +	active = kvm_active_vms;
> +	spin_unlock(&kvm_lock);
> +
> +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> +	if (pathbuf) {
> +		tmp = dentry_path_raw(kvm->debugfs_dentry,
> +				      pathbuf + sizeof(pathvar),
> +				      PATH_MAX - sizeof(pathvar));
> +		if (!IS_ERR(tmp)) {
> +			memcpy(tmp - sizeof(pathvar), pathvar, sizeof(pathvar));
> +			ptr[idx++] = tmp - sizeof(pathvar);
> +		}
> +	}
> +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);

Did you think about using struct kobj_uevent_env / add_uevent_var()?

But most probably the problem here is special handling for
dentry_path_raw().


-- 

Thanks,

David