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] [day] [month] [year] [list]

Message-ID: <d328ae8a-68c8-ac95-29c6-1c685ca1bfa4@redhat.com>
Date:   Mon, 10 Jul 2017 13:54:04 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Claudio Imbrenda <imbrenda@...ux.vnet.ibm.com>,
        David Hildenbrand <david@...hat.com>
Cc:     kvm@...r.kernel.org, borntraeger@...ibm.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 13:53, Claudio Imbrenda wrote:
> On Mon, 10 Jul 2017 11:40:55 +0200
> David Hildenbrand <david@...hat.com> wrote:
> 
>> On 10.07.2017 11:20, Claudio Imbrenda wrote:
>>
>> Minor minor nit:
>>
>> The subject should state "creating or destroying a VM"
> 
> I'll fix it

I can fix it when applying, no problem.

Paolo

> [...]
> 
>>> +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?
> 
> unfortunately yes. we don't have access to the pid when destroying the
> VM, so I take it from the debugfs entry, and that has the format
> "%d-%d", with pid and file descriptor, both can be 10 bytes long.
> 
>>> +	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()?
> 
> for what I could see, that only works with a more invasive patch. I'll
> see what I can do.
>  
>> But most probably the problem here is special handling for
>> dentry_path_raw().
>  
>