<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: Times New Roman; font-size: 12pt; color: #000000'><div><br></div><div> Hi,</div><div><br></div><div> I finally managed to commit the patch, after many detours and distractions. I decided to drop the 'SLICTable' alias because that's not been documented in a long time, but 'CustomTable' remains an alias for 'CustomTable0'. I've only given this minimal testing, once the commit hits the OSE tree please try it out and let me know if anything needs tweaking.</div><div><br></div><div> Regards,</div><div> Michal<br></div><div><br></div>----- Original Message -----<br>From: canardoscode@gmail.com<br>To: michal.necasek@oracle.com<br>Cc: vbox-dev@virtualbox.org<br>Sent: Saturday, September 22, 2018 11:42:59 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna<br>Subject: Re: [vbox-dev] PATCH: Implement support for multiple custom ACPI tables<br><br><div dir="ltr">Hi Michal,<div><br></div><div>>Do we really need three different names for one thing? SLICTable, CustomTable, CustomTable0 for the same thing seems like an overkill.</div><div><br></div><div>I agree. Ideally it would just be CustomTable0, CustomTable1, CustomTable2.....and the code currently supports that. I had kept SLICTable/CustomTable as aliases for CustomTable0 to maintain backward compatibility, expecting that at some point in the future they would likely be removed.</div><div><br></div><div>It's easy enough to remove the SLICTable/CustomTable aliases now if preferred. I would leave that decision to the devs, since it would be a breaking change for users currently using those config params.<br></div><div><br></div><div>Regards,</div><div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 18, 2018 at 3:56 PM Michal Necasek <<a href="mailto:michal.necasek@oracle.com" target="_blank">michal.necasek@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Hi,<br>
<br>
Sorry for the delay.<br>
<br>
You're absolutely right, the acpiR3SetupCust() function does a lot of <br>
things but acpiR3PhysCopy() is the only useful bit it does.<br>
<br>
The only objection I have then is the following: Do we really need <br>
three different names for one thing? SLICTable, CustomTable, <br>
CustomTable0 for the same thing seems like an overkill.<br>
<br>
Cheers,<br>
Michal<br>
<br>
On 9/7/2018 10:22 AM, Canardos . wrote:<br>
> Hi Michal,<br>
> <br>
> Thanks for taking a look.<br>
> <br>
> I agree that the behavior should not change and I don't believe it does. <br>
> I removed the call to acpiR3PrepareHeader (as well as the surrounding <br>
> lines) because it was dead code. Looking at the prior acpiR3SetupCust <br>
> function:<br>
> <br>
> /** Custom Description Table */<br>
> static void acpiR3SetupCust(ACPIState *pThis, RTGCPHYS32 addr)<br>
> {<br>
> ACPITBLCUST cust;<br>
> <br>
> /* First the ACPI version 1 version of the structure. */<br>
> memset(&cust, 0, sizeof(cust));<br>
> acpiR3PrepareHeader(pThis, &cust.header, "CUST", sizeof(cust), 1);<br>
> <br>
> memcpy(cust.header.au8OemTabId, pThis->au8OemTabId, 8);<br>
> cust.header.u32OemRevision = RT_H2LE_U32(pThis->u32OemRevision);<br>
> cust.header.u8Checksum = acpiR3Checksum((uint8_t *)&cust, <br>
> sizeof(cust));<br>
> <br>
> acpiR3PhysCopy(pThis, addr, pThis->pu8CustBin, pThis->cbCustBin);<br>
> }<br>
> <br>
> One can see that the cust structure is allocated locally on the first <br>
> line, setup, but then discarded when the function exits without ever <br>
> being used. The only code in the function that does anything is the <br>
> final line.<br>
> <br>
> The custom table header that is actually used is the one provided by the <br>
> user, which is loaded and present in the table itself <br>
> (pThis->pu8CustBin). This behavior has been retained in the updated <br>
> function:<br>
> <br>
> /** Custom Description Table */<br>
> static void acpiR3SetupCust(ACPIState *pThis, RTGCPHYS32 addr, uint8_t <br>
> custTableIdx)<br>
> {<br>
> Assert(custTableIdx < MAX_CUST_TABLES);<br>
> acpiR3PhysCopy(pThis, addr, pThis->pu8CustBin[custTableIdx], <br>
> pThis->cbCustBin[custTableIdx]);<br>
> }<br>
> <br>
> With respect to the incoming data, which part did you notice was copied <br>
> differently? The intention is definitely to retain the existing behavior <br>
> so if there's a difference there, it's unintended.<br>
> <br>
> In regard to use cases, one example is the requirement to now pass <br>
> through both SLIC and MSDM tables when using (legally) using a Windows <br>
> 10 OEM edition as a guest on the machine that it was licensed on (this <br>
> was the motivation for the original custom table mechanism for SLIC <br>
> passthrough for Windows 7/8 - see <br>
> <a href="https://www.virtualbox.org/ticket/9231" rel="noreferrer" target="_blank">https://www.virtualbox.org/ticket/9231</a>). Reducing the detection surface <br>
> for dynamic analysis of malware that actively detects sandboxes is another.<br>
> <br>
> Regards,<br>
> <br>
> On Tue, Sep 4, 2018 at 10:45 PM Michal Necasek <br>
> <<a href="mailto:michal.necasek@oracle.com" target="_blank">michal.necasek@oracle.com</a> <mailto:<a href="mailto:michal.necasek@oracle.com" target="_blank">michal.necasek@oracle.com</a>>> wrote:<br>
> <br>
> <br>
> Hi,<br>
> <br>
> Thanks for the patch! I quickly looked over it and it looks to me<br>
> that<br>
> the behavior of existing VMs with the<br>
> VBoxInternal/Devices/acpi/0/Config/CustomTable keys set will subtly<br>
> change because the call to<br>
> <br>
> acpiR3PrepareHeader(pThis, &cust.header, "CUST", sizeof(cust), 1);<br>
> <br>
> is no longer done and the incoming data is copied a bit differently.<br>
> Can<br>
> you explain why that's not done or why it can't cause any problems?<br>
> <br>
> Wouldn't it be better to keep the existing behavior for CustomTable<br>
> and just add CustomTable1/2/3 with slightly different behavior?<br>
> <br>
> Also, do you have a concrete example of ACPI tables that one might<br>
> want to pass to a VM this way?<br>
> <br>
> <br>
> Regards,<br>
> Michal<br>
> <br>
> On 8/24/2018 9:14 PM, Canardos . wrote:<br>
> > Hi Devs,<br>
> ><br>
> > This patch expands on work done several years ago, extending<br>
> support for<br>
> > custom ACPI tables from a single table to four tables, and<br>
> potentially N<br>
> > tables in future.<br>
> ><br>
> > The changes assist malware researchers to better emulate a physical<br>
> > machine (although increasing the aggregate allowable table size will<br>
> > assist further) as well as those with valid OEM licenses having<br>
> issues<br>
> > property passing through both SLIC and license tables.<br>
> ><br>
> > I limited the implementation to four custom tables given current<br>
> VBox<br>
> > limitations on the aggregate size of ACPI tables, but the code<br>
> changes<br>
> > support N tables.<br>
> ><br>
> > The new configuration keys are "CustomTable0..3". Legacy behavior<br>
> has<br>
> > been maintained, with any existing config entries for<br>
> "CustomTable", or<br>
> > "SLICTable" being used in the absence of a "CustomTable0" entry.<br>
> ><br>
> > The changes have been tested on Debian 9.5 (kernel 4.9.0-7/amd64)<br>
> and<br>
> > Manjaro (kernel 4.14.65-1/amd64) hosts with 32 and 64-bit Linux<br>
> guests.<br>
> ><br>
> > I've endeavored to reflect the code style of the file I was<br>
> working in,<br>
> > please let me know if anything is amiss.<br>
> ><br>
> > All attached code is released under the MIT License.<br>
> ><br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > vbox-dev mailing list<br>
> > <a href="mailto:vbox-dev@virtualbox.org" target="_blank">vbox-dev@virtualbox.org</a> <mailto:<a href="mailto:vbox-dev@virtualbox.org" target="_blank">vbox-dev@virtualbox.org</a>><br>
> > <a href="https://www.virtualbox.org/mailman/listinfo/vbox-dev" rel="noreferrer" target="_blank">https://www.virtualbox.org/mailman/listinfo/vbox-dev</a><br>
> ><br>
> <br>
> _______________________________________________<br>
> vbox-dev mailing list<br>
> <a href="mailto:vbox-dev@virtualbox.org" target="_blank">vbox-dev@virtualbox.org</a> <mailto:<a href="mailto:vbox-dev@virtualbox.org" target="_blank">vbox-dev@virtualbox.org</a>><br>
> <a href="https://www.virtualbox.org/mailman/listinfo/vbox-dev" rel="noreferrer" target="_blank">https://www.virtualbox.org/mailman/listinfo/vbox-dev</a><br>
> <br>
<br>
</blockquote></div>
</div></body></html>