Re: [PATCH v1 4/6] Sure Start Security Feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)

> 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.

>
> > +
> > +     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



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux