Re: [PATCH v10] isa-applesmc: provide OSK forwarding on Apple hosts

[Alexander Graf]

On 01.05.22 19:06, Alexander Graf wrote:
> Hey Vladislav,
>
> On 29.04.22 21:18, Vladislav Yaroshchuk wrote:
> > On Apple hosts we can read AppleSMC OSK key directly from host's
> > SMC and forward this value to QEMU Guest.
> >
> > New 'hostosk' property is added:
> > * `-device isa-applesmc,hostosk=on`
> > The property is set to 'on' by default for machine version > 7.0
> >
> > Apple licence allows use and run up to two additional copies
> > or instances of macOS operating system within virtual operating system
> > environments on each Apple-branded computer that is already running
> > the Apple Software, for purposes of:
> >   * software development
> >   * testing during software development
> >   * using macOS Server
> >   * personal, non-commercial use
> >
> > Guest macOS requires AppleSMC with correct OSK. The most legal
> > way to pass it to the Guest is to forward the key from host SMC
> > without any value exposion.
> >
> > Based on 
> > https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
> >
> > Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> > ---
> >   hw/core/machine.c  |   4 +-
> >   hw/misc/applesmc.c | 125 +++++++++++++++++++++++++++++++++++++++++++--
> >   2 files changed, 125 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index cb9bbc844d..7f4a27406a 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -37,7 +37,9 @@
> >   #include "hw/virtio/virtio.h"
> >   #include "hw/virtio/virtio-pci.h"
> >   -GlobalProperty hw_compat_7_0[] = {};
> > +GlobalProperty hw_compat_7_0[] = {
> > +    { "isa-applesmc", "hostosk", "off" }
> > +};
> >   const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
> >     GlobalProperty hw_compat_6_2[] = {
> > diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> > index 81cd6b6423..8672c9d56e 100644
> > --- a/hw/misc/applesmc.c
> > +++ b/hw/misc/applesmc.c
> > @@ -37,6 +37,11 @@
> >   #include "qemu/module.h"
> >   #include "qemu/timer.h"
> >   #include "qom/object.h"
> > +#include "qapi/error.h"
> > +
> > +#if defined(__APPLE__) && defined(__MACH__)
> > +#include <IOKit/IOKitLib.h>
> > +#endif
> >     /* #define DEBUG_SMC */
> >   @@ -80,7 +85,7 @@ enum {
> >   #define smc_debug(...) do { } while (0)
> >   #endif
> >   -static char default_osk[64] = "This is a dummy key. Enter the real 
> > key "
> > +static char default_osk[65] = "This is a dummy key. Enter the real 
> > key "
>
>
> This is only necessary if we run strlen() on the default_osk. Do we 
> have to?


Ugh, just after sending reply I realized why we need to: We make osk 
available as object property, which means anyone who would read it from 
outside would run out of bounds.

Can you please make the bit that bumps default_osk to 65 bytes a 
separate patch that we can apply immediately? It's a long standing, 
existing bug.


Thanks!

Alex