| 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
| ||
|
Date: Thu, 28 Jul 2022 09:53:58 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: kah.jing.lee@...el.com
Cc: linux-kernel@...r.kernel.org, arnd@...db.de,
rafael.j.wysocki@...el.com, tien.sung.ang@...el.com,
dinh.nguyen@...el.com, Zhou@...ah.com,
Furong <furong.zhou@...el.com>,
Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Subject: Re: [PATCH v2 1/3] drivers: misc: intel_sysid: Add sysid from arch
to drivers
On Thu, Jul 21, 2022 at 08:31:50PM +0800, kah.jing.lee@...el.com wrote:
> From: Kah Jing Lee <kah.jing.lee@...el.com>
>
> Add new sysid driver. The Altera(Intel) Sysid component is generally part
> of an FPGA design. The component can be hotplugged when the FPGA is
> reconfigured. This driver support the component being hotplugged.
> The sysid driver stores unique 32-bit id value which is similar to a
> check-sum value; different components, different configuration options,
> or both, can be configured to produce different id values. Timestamp field
> is the unique 32-bit value that is based on the system generation time.
>
> There are two basic ways to use the system ID core:
> - Verify the system ID before downloading new software to a system. This
> method can be used by software development tools, before downloading a
> program to a target hardware system, if the program is compiled for
> different hardware.
>
> - Check system ID after reset. If a program is running on hardware other
> than the expected Platform Designer system, the program may fail to
> function altogether. If the program does not crash, it can behave
> erroneously in subtle ways that are difficult to debug. To protect against
> this case, a program can compare the expected system ID against the system
> ID core, and report an error if they do not match.
>
> Usage:
> cat /sys/bus/platform/devices/soc:base_fpga_region/
> soc:base_fpga_region:fpga_pr_region0/[addr.sysid]/sysid/id
> cat /sys/bus/platform/devices/soc:base_fpga_region/
> soc:base_fpga_region:fpga_pr_region0/[addr.sysid]/sysid/buildtime
>
> Based on an initial contribution from Ley Foon Tan at Altera
> Signed-off-by: Kah Jing Lee <kah.jing.lee@...el.com>
> Reviewed-by: Zhou, Furong <furong.zhou@...el.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> ---
> v2:
> - Updated license header, commit message and author since original author
> no longer here
> - Updated driver description
> - Removed redundant word in Kconfig
> - Updated timestamp function, renamed timestamp -> buildtime due to programmed
> time during design generation instead of real-time timestamp reading
> - Updated Kconfig description
> - Updated sysfs add to devm_add_group
> - Add the Documentation/ABI/testing file
> - Updated header and newline formatting
> ---
> ---
> drivers/misc/Kconfig | 9 +++
> drivers/misc/Makefile | 1 +
> drivers/misc/intel_sysid.c | 114 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 124 insertions(+)
> create mode 100644 drivers/misc/intel_sysid.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 41d2bb0ae23a..30cf36916593 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -483,6 +483,15 @@ config OPEN_DICE
>
> If unsure, say N.
>
> +config INTEL_SYSID
> + tristate "Intel System ID"
> + help
> + This enables the Intel System ID driver for a soft core.
> + Say Y here if you want to build a driver for Intel System ID.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called intel_sysid. If unsure, say N here.
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 70e800e9127f..301c854b4cd3 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCH_PHUB) += pch_phub.o
> obj-y += ti-st/
> obj-y += lis3lv02d/
> obj-$(CONFIG_ALTERA_STAPL) +=altera-stapl/
> +obj-$(CONFIG_INTEL_SYSID) += intel_sysid.o
> obj-$(CONFIG_INTEL_MEI) += mei/
> obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
> obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
> diff --git a/drivers/misc/intel_sysid.c b/drivers/misc/intel_sysid.c
> new file mode 100644
> index 000000000000..b63dbde63347
> --- /dev/null
> +++ b/drivers/misc/intel_sysid.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022, Intel Corporation.
> + * Copyright (C) 2013-2015, Altera Corporation.
> + *
> + * Ley Foon Tan <lftan@...era.com>
> + * Kah Jing Lee <kah.jing.lee@...el.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define DRV_NAME "intel_sysid"
KBUILD_MODNAME?
> +
> +struct intel_sysid {
> + void __iomem *regs;
> +};
> +
> +/* System ID Registers*/
> +#define SYSID_REG_ID 0x0
> +#define SYSID_REG_TIMESTAMP 0x4
> +
> +static ssize_t id_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct intel_sysid *sysid = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", readl(sysid->regs + SYSID_REG_ID));
sysfs_emit() please.
> +}
> +
> +static void convert_readable_timestamp(struct tm *buildtime)
> +{
> + buildtime->tm_year += 1900;
> + buildtime->tm_mon += 1;
> +}
> +
> +static ssize_t buildtime_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + unsigned int reg;
> + struct tm buildtime;
> + struct intel_sysid *sysid = dev_get_drvdata(dev);
> +
> + reg = readl(sysid->regs + SYSID_REG_TIMESTAMP);
> + time64_to_tm(reg, 0, &buildtime);
> + convert_readable_timestamp(&buildtime);
> +
> + return sprintf(buf, "%u (%u-%u-%u %u:%u:%u UTC)\n", reg,
> + (unsigned int)(buildtime.tm_year),
> + buildtime.tm_mon, buildtime.tm_mday, buildtime.tm_hour,
> + buildtime.tm_min, buildtime.tm_sec);
As said in the documentation review, use a standard format, do not make
up a new one.
> +}
> +
> +static DEVICE_ATTR_RO(id);
> +static DEVICE_ATTR_RO(buildtime);
> +
> +static struct attribute *intel_sysid_attrs[] = {
> + &dev_attr_id.attr,
> + &dev_attr_buildtime.attr,
> + NULL
> +};
> +
> +struct attribute_group intel_sysid_attr_group = {
> + .name = "sysid",
> + .attrs = intel_sysid_attrs,
> +};
> +
> +static int intel_sysid_probe(struct platform_device *pdev)
> +{
> + struct intel_sysid *sysid;
> + struct resource *regs;
> +
> + sysid = devm_kzalloc(&pdev->dev, sizeof(struct intel_sysid),
> + GFP_KERNEL);
> + if (!sysid)
> + return -ENOMEM;
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs)
> + return -ENXIO;
> +
> + sysid->regs = devm_ioremap_resource(&pdev->dev, regs);
> + if (IS_ERR(sysid->regs))
> + return PTR_ERR(sysid->regs);
> +
> + platform_set_drvdata(pdev, sysid);
> +
> + return devm_device_add_group(&pdev->dev, &intel_sysid_attr_group);
You just raced with userspace and lost. Please use the default group
for the platform device.
I need to go remove this function, it should not be used at all as it is
broken.
thanks,
greg k-h
Powered by blists - more mailing lists