[Public]
> -----Original Message-----
> From: Jorge Lopez <jorgealtxwork@xxxxxxxxx>
> Sent: Tuesday, April 5, 2022 11:56
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Cc: platform-driver-x86@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v1 4/6] Sure Start Security Feature
>
> hi Mario,
>
> On Mon, Apr 4, 2022 at 5:05 PM Limonciello, Mario
> <Mario.Limonciello@xxxxxxx> wrote:
> >
> > [Public]
> >
> >
> >
> > > -----Original Message-----
> > > From: Jorge Lopez <jorgealtxwork@xxxxxxxxx>
> > > Sent: Monday, April 4, 2022 15:36
> > > To: platform-driver-x86@xxxxxxxxxxxxxxx
> > > Subject: [PATCH v1 4/6] Sure Start Security Feature
> > >
> > > Sure Start provides advanced firmware protection and resiliency by
> > > identifying and repairing unauthorized BIOS changes. It maintains an
> > > audit log of these events and other important system configuration
> > > changes. The following sysfs entries can be used to read the contents
> > > of the audit log.
> > >
> > > /sys/devices/platform/hp-wmi/sure_start/audit_log_entry_count
> > > /sys/devices/platform/hp-wmi/sure_start/audit_log_entries
> > >
> > > 'audit_log_entry_count' is a read-only file that returns the number of
> > > existing audit log events available to be read
> > >
> > > 'audit_log_entries' is a read-only file that returns the events in the
> > > log
> > >
> > > This feature requires "Update hp_wmi_group to simplify feature
> > > addition" patch.
> > >
> > > All changes were validated on a HP ZBook Workstation,
> > > HP EliteBook x360, and HP EliteBook 850 G8 notebooks.
> > >
> > > Signed-off-by: Jorge Lopez <jorge.lopez2@xxxxxx>
> > >
> > > ---
> > > Based on the latest platform-drivers-x86.git/for-next
> > > ---
> > > drivers/platform/x86/hp-wmi.c | 108
> > > ++++++++++++++++++++++++++++++++++
> > > 1 file changed, 108 insertions(+)
> > >
> > > diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-
> wmi.c
> > > index 139dc079c1fa..918e3eaf1b67 100644
> > > --- a/drivers/platform/x86/hp-wmi.c
> > > +++ b/drivers/platform/x86/hp-wmi.c
> > > @@ -126,6 +126,11 @@ enum hp_wmi_spm_commandtype {
> > > HPWMI_SECUREPLATFORM_SET_SK = 0x12,
> > > };
> > >
> > > +enum hp_wmi_surestart_commandtype {
> > > + HPWMI_SURESTART_GET_LOG_COUNT = 0x01,
> > > + HPWMI_SURESTART_GET_LOG = 0x02,
> > > +};
> > > +
> > > enum hp_wmi_gm_commandtype {
> > > HPWMI_FAN_SPEED_GET_QUERY = 0x11,
> > > HPWMI_SET_PERFORMANCE_MODE = 0x1A,
> > > @@ -138,6 +143,7 @@ enum hp_wmi_command {
> > > HPWMI_READ = 0x01,
> > > HPWMI_WRITE = 0x02,
> > > HPWMI_ODM = 0x03,
> > > + HPWMI_SURESTART = 0x20006,
> > > HPWMI_GM = 0x20008,
> > > HPWMI_SECUREPLATFORM = 0x20010,
> > > };
> > > @@ -851,6 +857,7 @@ static ssize_t spm_kek_store(struct kobject *kobj,
> > > {
> > > int ret =
> > > hp_wmi_perform_query(HPWMI_SECUREPLATFORM_SET_KEK,
> > > HPWMI_SECUREPLATFORM, (void *)buf,
> > > count, 0);
> > > +
> > > return ret ? -EINVAL : count;
> > > }
> > >
> > > @@ -918,6 +925,106 @@ static const struct attribute_group spm_group =
> {
> > > .attrs = spm_attrs,
> > > };
> > >
> > > +/* Sure Start functions */
> > > +
> > > +#define LOG_MAX_ENTRIES 254
> > > +#define LOG_ENTRY_SIZE 16
> > > +
> > > +/*
> > > + * sure_start_audit_log_entry_count_show - Reports the number of
> > > + * existing audit log entries available
> > > + * to be read
> > > + *
> > > + * @kobj: Pointer to a kernel object of things that show up as directory
> > > + * in the sysfs filesystem.
> > > + * @attr: Pointer to list of attributes for the operation
> > > + * @buf: Pointer to buffer
> > > + *
> > > + * Returns number of existing audit log entries available to be read,
> > > + * audit log entry size, and maximum number of entries
> > > + * supported. Otherwise, an HP WMI query specific error code
> > > + * (which is negative)
> > > + *
> > > + * [No of entries],[log entry size],[Max number of entries
> supported]
> > > + */
> > > +static ssize_t sure_start_audit_log_entry_count_show(struct kobject
> > > *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buf)
> > > +{
> > > + int ret;
> > > + u32 count = 0;
> > > +
> > > + ret =
> > > hp_wmi_perform_query(HPWMI_SURESTART_GET_LOG_COUNT,
> > > HPWMI_SURESTART,
> > > + &count, 0, sizeof(count));
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return snprintf(buf, PAGE_SIZE, "%d,%d,%d\n", count,
> > > LOG_ENTRY_SIZE, LOG_MAX_ENTRIES);
> > > +}
> > > +
> > > +/*
> > > + * sure_start_audit_log_entries_show() - Return all entries found in log
> file
> > > + *
> > > + * @kobj: Pointer to a kernel object of things that show up as
> > > + * directory in the sysfs filesystem.
> > > + * @attr: Pointer to list of attributes for the operation
> > > + * @buf: Pointer to buffer
> > > + *
> > > + * Returns number of bytes needed to read all audit logs entries to be
> read.
> > > + * Otherwise, an HP WMI query specific error code (which is
> negative)
> > > + * -EFAULT if the audit logs size exceeds 4KB
> > > + *
> > > + */
> > > +static ssize_t sure_start_audit_log_entries_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buf)
> > > +{
> > > + int ret;
> > > + int i;
> > > + u32 count = 0;
> > > +
> > > + // Get the number of event logs
> > > + ret =
> > > hp_wmi_perform_query(HPWMI_SURESTART_GET_LOG_COUNT,
> > > HPWMI_SURESTART,
> > > + &count, 1, 4);
> > > +
> > > + /*
> > > + * The show() api will not work if the audit logs ever go
> > > + * beyond 4KB
> > > + */
> > > + if (count * LOG_ENTRY_SIZE > PAGE_SIZE)
> > > + return -EFAULT;
> >
> > This is an interface that will be there forever, what are realistic numbers
> and sizes after a long time?
> > I have an AMD HP laptop that is only been used a few months and checked
> the size like this:
> > dd if=audit_log_entries of=/tmp/entries
> >
> The audit log will grow linearly and it is defined by BIOS. The total
> number of bytes ( audit log size times the max number of audit logs
> supported by BIOS) is 4K.
> The audit log size reports three values. Number of event logs, audit
> log max size (16), and max of audit logs supported by BIOS (254).
> (16*254 = 4K).
> In your case, the total number of bytes for all audit logs available
> is 224 bytes. You can conclude, your system has 14 audit logs
> available to be read (224/16 = 14)
But so if this is growing linearly what happens if BIOS ends up with
more than 4k audit logs? Can that happen?
If that happens I can no longer access them from userspace right?
>
> > If so, then maybe this should be designed as a different interface.
> >
> > Also, the log is readable by anybody. Should this be root only?
>
> No. audit logs will be available to everyone. The interpretation of
> each audit event is done at the application level.
Got it - thanks
>
> >
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /*
> > > + * We are guaranteed the buffer is 4KB so today all the event
> > > + * logs will fit
> > > + */
> > > + for (i = 0; ((i < count) & (ret >= 0)); i++) {
> > > + *buf = (i + 1);
> > > + ret =
> > > hp_wmi_perform_query(HPWMI_SURESTART_GET_LOG,
> > > + HPWMI_SURESTART,
> > > + buf, 1, 128);
> > > + if (ret >= 0)
> > > + buf += LOG_ENTRY_SIZE;
> > > + }
> > > +
> > > + return (count * LOG_ENTRY_SIZE);
> > > +}
> > > +
> > > +HPWMI_ATTR_RO(sure_start, audit_log_entry_count);
> > > +HPWMI_ATTR_RO(sure_start, audit_log_entries);
> > > +
> > > +static struct attribute *sure_start_attrs[] = {
> > > + &sure_start_audit_log_entry_count.attr,
> > > + &sure_start_audit_log_entries.attr,
> > > + NULL,
> > > +};
> > > +
> > > +static const struct attribute_group sure_start_group = {
> > > + .name = "sure_start",
> > > + .attrs = sure_start_attrs,
> > > +};
> > > +
> > > static DEVICE_ATTR_RO(display);
> > > static DEVICE_ATTR_RO(hddtemp);
> > > static DEVICE_ATTR_RW(als);
> > > @@ -942,6 +1049,7 @@ static const struct attribute_group
> hp_wmi_group =
> > > {
> > > static const struct attribute_group *hp_wmi_groups[] = {
> > > &hp_wmi_group,
> > > &spm_group,
> > > + &sure_start_group,
> > > NULL,
> > > };
> > >
> > > --
> > > 2.25.1
