linux-kernel - Re: [PATCH 3/3] platform/x86: Intel PMT Crashlog capability driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 19 Sep 2020 10:58:38 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     "David E. Box" <david.e.box@...ux.intel.com>, lee.jones@...aro.org,
        dvhart@...radead.org, andy@...radead.org,
        alexander.h.duyck@...ux.intel.com
Cc:     linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        Alexey Budankov <alexey.budankov@...ux.intel.com>
Subject: Re: [PATCH 3/3] platform/x86: Intel PMT Crashlog capability driver

Hi,

Thanks for the patches.

On 11.09.2020 22:45, David E. Box wrote:
> From: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
> 
> Add support for the Intel Platform Monitoring Technology crashlog
> interface.  This interface provides a few sysfs values to allow for
> controlling the crashlog telemetry interface as well as a character driver
> to allow for mapping the crashlog memory region so that it can be accessed
> after a crashlog has been recorded.
> 
> This driver is meant to only support the server version of the crashlog
> which is identified as crash_type 1 with a version of zero. Currently no
> other types are supported.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
> Signed-off-by: David E. Box <david.e.box@...ux.intel.com>
> ---
>  .../ABI/testing/sysfs-class-pmt_crashlog      |  66 ++
>  drivers/platform/x86/Kconfig                  |  10 +
>  drivers/platform/x86/Makefile                 |   1 +
>  drivers/platform/x86/intel_pmt_crashlog.c     | 588 ++++++++++++++++++
>  4 files changed, 665 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-pmt_crashlog
>  create mode 100644 drivers/platform/x86/intel_pmt_crashlog.c

<SNIP>

> +
> +/*
> + * devfs
> + */
> +static int pmt_crashlog_open(struct inode *inode, struct file *filp)
> +{
> +	struct crashlog_entry *entry;
> +	struct pci_driver *pci_drv;
> +	struct pmt_crashlog_priv *priv;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;

Will not this above still block access to /dev/crashlogX for admin_group users
in case root configured access e.g. similar to this:

ls -alh /dev/
crw-rw----.  1 root admin_group      1,   9 Sep 15 18:28 crashlogX

If yes then that capable() check is probably superfluous and
should be avoided in order not to block access to PMT data.

Could you please clarify or comment?

Thanks,
Alexei