| 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
| ||
|
Date: Sun, 15 Aug 2021 23:21:14 +0800
From: Tianyu Lan <ltykernel@...il.com>
To: Michael Kelley <mikelley@...rosoft.com>,
KY Srinivasan <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
"hpa@...or.com" <hpa@...or.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"luto@...nel.org" <luto@...nel.org>,
"peterz@...radead.org" <peterz@...radead.org>,
"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
"boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
"jgross@...e.com" <jgross@...e.com>,
"sstabellini@...nel.org" <sstabellini@...nel.org>,
"joro@...tes.org" <joro@...tes.org>,
"will@...nel.org" <will@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
"arnd@...db.de" <arnd@...db.de>, "hch@....de" <hch@....de>,
"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
"robin.murphy@....com" <robin.murphy@....com>,
"thomas.lendacky@....com" <thomas.lendacky@....com>,
"brijesh.singh@....com" <brijesh.singh@....com>,
"ardb@...nel.org" <ardb@...nel.org>,
Tianyu Lan <Tianyu.Lan@...rosoft.com>,
"pgonda@...gle.com" <pgonda@...gle.com>,
"martin.b.radev@...il.com" <martin.b.radev@...il.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"rppt@...nel.org" <rppt@...nel.org>,
"sfr@...b.auug.org.au" <sfr@...b.auug.org.au>,
"saravanand@...com" <saravanand@...com>,
"krish.sadhukhan@...cle.com" <krish.sadhukhan@...cle.com>,
"aneesh.kumar@...ux.ibm.com" <aneesh.kumar@...ux.ibm.com>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"rientjes@...gle.com" <rientjes@...gle.com>,
"hannes@...xchg.org" <hannes@...xchg.org>,
"tj@...nel.org" <tj@...nel.org>
Cc: "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
vkuznets <vkuznets@...hat.com>,
"parri.andrea@...il.com" <parri.andrea@...il.com>,
"dave.hansen@...el.com" <dave.hansen@...el.com>
Subject: Re: [PATCH V3 04/13] HV: Mark vmbus ring buffer visible to host in
Isolation VM
On 8/13/2021 6:20 AM, Michael Kelley wrote:
>> @@ -474,6 +482,13 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>> if (ret)
>> return ret;
>>
>> + ret = set_memory_decrypted((unsigned long)kbuffer,
>> + HVPFN_UP(size));
>> + if (ret) {
>> + pr_warn("Failed to set host visibility.\n");
> Enhance this message a bit. "Failed to set host visibility for new GPADL\n"
> and also output the value of ret.
OK. This looks better. Thanks.
>
>> @@ -539,6 +554,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>> /* At this point, we received the gpadl created msg */
>> *gpadl_handle = gpadlmsg->gpadl;
>>
>> + channel->gpadl_array[index].size = size;
>> + channel->gpadl_array[index].buffer = kbuffer;
>> + channel->gpadl_array[index].gpadlhandle = *gpadl_handle;
>> +
> I can see the merits of transparently stashing the memory address and size
> that will be needed by vmbus_teardown_gpadl(), so that the callers of
> __vmbus_establish_gpadl() don't have to worry about it. But doing the
> stashing transparently is somewhat messy.
>
> Given that the callers are already have memory allocated to save the
> GPADL handle, a little refactoring would make for a much cleaner solution.
> Instead of having memory allocated for the 32-bit GPADL handle, callers
> should allocate the slightly larger struct vmbus_gpadl that you've
> defined below. The calling interfaces can be updated to take a pointer
> to this structure instead of a pointer to the 32-bit GPADL handle, and
> you can save the memory address and size right along with the GPADL
> handle. This approach touches a few more files, but I think there are
> only two callers outside of the channel management code -- netvsc
> and hv_uio -- so it's not a big change.
Yes, this is a good suggestion and Will update in the next version.
>
>> @@ -859,6 +886,19 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
>> spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
>>
>> kfree(info);
>> +
>> + /* Find gpadl buffer virtual address and size. */
>> + for (i = 0; i < VMBUS_GPADL_RANGE_COUNT; i++)
>> + if (channel->gpadl_array[i].gpadlhandle == gpadl_handle)
>> + break;
>> +
>> + if (set_memory_encrypted((unsigned long)channel->gpadl_array[i].buffer,
>> + HVPFN_UP(channel->gpadl_array[i].size)))
>> + pr_warn("Fail to set mem host visibility.\n");
> Enhance this message a bit: "Failed to set host visibility in GPADL teardown\n".
> Also output the returned error code to help in debugging any occurrences of
> a failure
Yes, agree. Will update.
Thanks.
Powered by blists - more mailing lists