linux-kernel - Re: Re: [PATCH v17 12/35] virt: gunyah: Add resource tickets

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]

Message-ID: <20240311100247856-0700.eberman@hu-eberman-lv.qualcomm.com>
Date: Mon, 11 Mar 2024 10:13:03 -0700
From: Elliot Berman <quic_eberman@...cinc.com>
To: Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>
CC: Alex Elder <elder@...aro.org>,
        Srinivas Kandagatla
	<srinivas.kandagatla@...aro.org>,
        Murali Nalajal <quic_mnalajal@...cinc.com>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Carl van Schaik
	<quic_cvanscha@...cinc.com>,
        Philip Derrin <quic_pderrin@...cinc.com>,
        Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
        Jonathan Corbet
	<corbet@....net>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        "Fuad
 Tabba" <tabba@...gle.com>,
        Sean Christopherson <seanjc@...gle.com>,
        "Andrew
 Morton" <akpm@...ux-foundation.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-mm@...ck.org>
Subject: Re: Re: [PATCH v17 12/35] virt: gunyah: Add resource tickets

On Mon, Mar 11, 2024 at 11:08:06AM +0530, Srivatsa Vaddagiri wrote:
> * Elliot Berman <quic_eberman@...cinc.com> [2024-02-22 15:16:35]:
> 
> > Gunyah doesn't process the label and that makes it
> > possible for userspace to create multiple resources with the same label.
> 
> I think that description conflicts with what is implemented in this patch?
> 
> int gunyah_vm_add_resource_ticket(struct gunyah_vm *ghvm,
> 				  struct gunyah_vm_resource_ticket *ticket)
> {
> 	mutex_lock(&ghvm->resources_lock);
> 	list_for_each_entry(iter, &ghvm->resource_tickets, vm_list) {
> 		if (iter->resource_type == ticket->resource_type &&
> 		    iter->label == ticket->label) {
> 			ret = -EEXIST;
> 			goto out;
> 		}
> 	}
> 
> 
> //
> 

It's a justification for the next sentence in the commit text:

    Resource ticket owners need to be prepared for populate to be called
    multiple times if userspace created multiple resources with the same
    label.


VM manager can make sure that only one entity is going to receive the
resources (the check above you highlighted), but I can't make sure that
it's only going to exactly one resource. We don't currently have a
scenario where we need/want multiple resources with the same label, we
might have that in the future and I didn't want to add that restriction
in common code.

> > @@ -134,6 +246,25 @@ static int gunyah_vm_start(struct gunyah_vm *ghvm)
> >  	}
> >  	ghvm->vm_status = GUNYAH_RM_VM_STATUS_READY;
> >  
> > +	ret = gunyah_rm_get_hyp_resources(ghvm->rm, ghvm->vmid, &resources);
> > +	if (ret) {
> > +		dev_warn(ghvm->parent,
> > +			 "Failed to get hypervisor resources for VM: %d\n",
> > +			 ret);
> > +		goto err;
> > +	}
> 
> Where do we free memory pointed by 'resources' ptr?
> 

Ah, I meant to add the __free(kfree) annotation. I've added it.

> > +
> > +	for (i = 0, n = le32_to_cpu(resources->n_entries); i < n; i++) {
> > +		ghrsc = gunyah_rm_alloc_resource(ghvm->rm,
> > +						 &resources->entries[i]);
> > +		if (!ghrsc) {
> > +			ret = -ENOMEM;
> > +			goto err;
> > +		}
> > +
> > +		gunyah_vm_add_resource(ghvm, ghrsc);
> > +	}