| 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
| ||
|
Message-ID: <CAKMK7uEYKwXgDPMkA632UVX7+eN5vS6_EJqHYNnEDDg10O68zg@mail.gmail.com>
Date: Wed, 3 Sep 2014 16:19:34 +0200
From: Daniel Vetter <daniel.vetter@...ll.ch>
To: Johannes Berg <johannes@...solutions.net>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Seth Forshee <seth.forshee@...onical.com>,
Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
luca@...lho.fi, kvalo@...rom.com
Subject: Re: [RFC] firmware coredump: add new firmware coredump class
[super-embarrassing resend, the previous one contained html gunk.]
If the idea is to also convert gpu crash dumps to this we should add
dri-devel. And there the crashes are usually not due to firmware, but
because the shaders and command batches userspace submitted have
issues, so this should also be renamed to dev_coredump I think.
On the overall design I wonder whether this shouldn't work more like a
real core dump and dump to a real file. At least currently the dumps
i915 creates are only useful as a general guide to where things went
wrong, but if we actually want to submit them as traces to the
hardware people we need to dump a _lot_ more. Otoh with the future of
shared virtual address spaces between gpu/cpu we might just do a real
core dump, so maybe this use case should be out of scope for your
patch here.
On the logic itself I'm not sure whether the timeout is all that
useful - at least in i915 our crash recovery works well enough that
reporters often don't realize right away when it happened, but only
later on when looking through logs to explain the tiny corruptions. If
the crashdupm has evapored meanwhile that's not that useful.
Also, at least for gpus it's usually not interesting to grab
subsequent dumps: Often the gpu is in a bad mood due to the first
crash, or it's just a massive row of duplicated dumps. So in i915 we
only record the first crash and keep it around forever. And tooling
can still free it by writing to the file. This also ensures that we
don't waste excessive amounts of memory with crash dumps.
And if we want to use this for i915 we need some way for tools to go
from the i915 drm class device node to the error state, not just from
the error state back to the device.
-Daniel
On Wed, Sep 3, 2014 at 1:15 PM, Johannes Berg <johannes@...solutions.net> wrote:
> From: Johannes Berg <johannes.berg@...el.com>
>
> Many devices run firmware, and as other software such firmware has
> bugs. When it misbehaves, however, it is often much harder to debug
> than software running on the host.
>
> Introduce a "firmware coredump" mechanism to allow dumping internal
> firmware state through a generalized mechanism. As all devices are
> different and information needed can vary accordingly, this doesn't
> prescribe a file format - it just provides mechanism to get data to
> be able to capture it in a generalized way (e.g. in distributions.)
>
> Note that generalized capturing of such data may result in privacy
> issues, so users generally need to be involved. In order to allow
> certain users/system integrators/... to disable the feature at all,
> introduce a Kconfig option to override the drivers that would like
> to have the feature.
>
> For now, this provides two ways of dumping data:
> 1) with a vmalloc'ed area, that is then given to the fwcoredump
> subsystem and freed after retrieval or timeout
> 2) with a generalized reader/free function method
>
> We could/should add more options, e.g. a list of pages, since the
> vmalloc area is very limited on some architectures.
>
> Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> ---
> MAINTAINERS | 7 ++
> drivers/base/Kconfig | 21 +++++
> drivers/base/Makefile | 1 +
> drivers/base/fwcoredump.c | 222 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/fwcoredump.h | 35 +++++++
> 5 files changed, 286 insertions(+)
> create mode 100644 drivers/base/fwcoredump.c
> create mode 100644 include/linux/fwcoredump.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2f85f55c8fb8..394bda1cde52 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3729,6 +3729,13 @@ F: Documentation/firmware_class/
> F: drivers/base/firmware*.c
> F: include/linux/firmware.h
>
> +FIRMWARE COREDUMP (fwcoredump)
> +M: Johannes Berg <johannes@...solutions.net>
> +L: linux-kernel@...r.kernel.org
> +S: Maintained
> +F: drivers/base/fwcoredump.c
> +F: include/linux/fwcoredump.h
> +
> FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
> M: Joshua Morris <josh.h.morris@...ibm.com>
> M: Philip Kelleher <pjk1939@...ux.vnet.ibm.com>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 4e7f0ff83ae7..31eabab20c8a 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -165,6 +165,27 @@ config FW_LOADER_USER_HELPER_FALLBACK
>
> If you are unsure about this, say N here.
>
> +config WANT_FW_COREDUMP
> + bool
> + help
> + Drivers should "select" this option if they desire to use the
> + firmware coredump mechanism.
> +
> +config DISABLE_FW_COREDUMP
> + bool "Disable firmware coredump" if EXPERT
> + help
> + Disable the firmware coredump mechanism despite drivers wanting to
> + use it; this allows for more sensitive systems or systems that
> + don't want to ever access the information to not have the code,
> + nor keep any data.
> +
> + If unsure, say N.
> +
> +config FW_COREDUMP
> + bool
> + default y if WANT_FW_COREDUMP
> + depends on !DISABLE_FW_COREDUMP
> +
> config DEBUG_DRIVER
> bool "Driver Core verbose debug messages"
> depends on DEBUG_KERNEL
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 4aab26ec0292..2af1be519653 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
> obj-$(CONFIG_REGMAP) += regmap/
> obj-$(CONFIG_SOC_BUS) += soc.o
> obj-$(CONFIG_PINCTRL) += pinctrl.o
> +obj-$(CONFIG_FW_COREDUMP) += fwcoredump.o
>
> ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
>
> diff --git a/drivers/base/fwcoredump.c b/drivers/base/fwcoredump.c
> new file mode 100644
> index 000000000000..f70bef0727a9
> --- /dev/null
> +++ b/drivers/base/fwcoredump.c
> @@ -0,0 +1,222 @@
> +/******************************************************************************
> + *
> + * This file is provided under the GPLv2 license.
> + *
> + * GPL LICENSE SUMMARY
> + *
> + * Copyright(c) 2014 Intel Mobile Communications GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * The full GNU General Public License is included in this distribution
> + * in the file called COPYING.
> + *
> + * Contact Information:
> + * Intel Linux Wireless <ilw@...ux.intel.com>
> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
> + */
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/fwcoredump.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/workqueue.h>
> +
> +MODULE_AUTHOR("Johannes Berg <johannes@...solutions.net>");
> +MODULE_DESCRIPTION("Firmware Coredump support");
> +MODULE_LICENSE("GPL");
> +
> +/* if data isn't read by userspace after 5 minutes delete it */
> +#define FWC_TIMEOUT (HZ * 60 * 5)
> +
> +struct fwc_entry {
> + struct device fwc_dev;
> + const void *data;
> + size_t datalen;
> + struct module *owner;
> + ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> + const void *data, size_t datalen);
> + void (*free)(const void *data);
> + struct delayed_work del_wk;
> +};
> +
> +static struct fwc_entry *dev_to_fwc(struct device *dev)
> +{
> + return container_of(dev, struct fwc_entry, fwc_dev);
> +}
> +
> +static void fwc_dev_release(struct device *dev)
> +{
> + struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> + fwc->free(fwc->data);
> + kfree(fwc);
> +}
> +
> +static void fwc_del(struct work_struct *wk)
> +{
> + struct fwc_entry *fwc;
> +
> + fwc = container_of(wk, struct fwc_entry, del_wk.work);
> +
> + device_del(&fwc->fwc_dev);
> + put_device(&fwc->fwc_dev);
> +}
> +
> +static ssize_t fwc_data_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr,
> + char *buffer, loff_t offset, size_t count)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> + return fwc->read(buffer, offset, count, fwc->data, fwc->datalen);
> +}
> +
> +static ssize_t fwc_data_write(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr,
> + char *buffer, loff_t offset, size_t count)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> + schedule_delayed_work(&fwc->del_wk, 0);
> +
> + return count;
> +}
> +
> +static struct bin_attribute fwc_attr_data = {
> + .attr = { .name = "data", .mode = S_IRUSR | S_IWUSR, },
> + .size = 0,
> + .read = fwc_data_read,
> + .write = fwc_data_write,
> +};
> +
> +static struct bin_attribute *fwc_dev_bin_attrs[] = {
> + &fwc_attr_data, NULL,
> +};
> +
> +static const struct attribute_group fwc_dev_group = {
> + .bin_attrs = fwc_dev_bin_attrs,
> +};
> +
> +static const struct attribute_group *fwc_dev_groups[] = {
> + &fwc_dev_group, NULL,
> +};
> +
> +static struct class fwc_class = {
> + .name = "fwcoredump",
> + .dev_release = fwc_dev_release,
> + .dev_groups = fwc_dev_groups,
> +};
> +
> +static ssize_t fwc_readv(char *buffer, loff_t offset, size_t count,
> + const void *data, size_t datalen)
> +{
> + if (offset > datalen)
> + return -EINVAL;
> +
> + if (offset + count > datalen)
> + count = datalen - offset;
> +
> + if (count)
> + memcpy(buffer, ((u8 *)data) + offset, count);
> +
> + return count;
> +}
> +
> +/**
> + * fw_coredumpv - create firmware coredump with vmalloc data
> + * @dev: the struct device for the crashed device
> + * @data: vmalloc data containing the firmware coredump
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + */
> +void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
> + gfp_t gfp)
> +{
> + fw_coredumpm(dev, NULL, data, datalen, gfp, fwc_readv, vfree);
> +}
> +EXPORT_SYMBOL(fw_coredumpv);
> +
> +/**
> + * fw_coredumpm - create firmware coredump with read/free methods
> + * @dev: the struct device for the crashed device
> + * @data: data cookie for the @read/@...e functions
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + * @read: function to read from the given buffer
> + * @free: function to free the given buffer
> + */
> +void fw_coredumpm(struct device *dev, struct module *owner,
> + const void *data, size_t datalen, gfp_t gfp,
> + ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> + const void *data, size_t datalen),
> + void (*free)(const void *data))
> +{
> + static atomic_t fwc_count = ATOMIC_INIT(0);
> + struct fwc_entry *fwc;
> +
> + if (!try_module_get(owner))
> + return;
> +
> + fwc = kzalloc(sizeof(*fwc), gfp);
> + if (!fwc)
> + goto put_module;
> +
> + fwc->owner = owner;
> + fwc->data = data;
> + fwc->datalen = datalen;
> + fwc->read = read;
> + fwc->free = free;
> +
> + device_initialize(&fwc->fwc_dev);
> +
> + dev_set_name(&fwc->fwc_dev, "fwc%d", atomic_inc_return(&fwc_count));
> + fwc->fwc_dev.class = &fwc_class;
> +
> + if (device_add(&fwc->fwc_dev))
> + goto put_device;
> +
> + if (sysfs_create_link(&fwc->fwc_dev.kobj, &dev->kobj, "failing_device"))
> + /* nothing - symlink will be missing but that's ok */;
> +
> + INIT_DELAYED_WORK(&fwc->del_wk, fwc_del);
> + schedule_delayed_work(&fwc->del_wk, FWC_TIMEOUT);
> +
> + return;
> + put_device:
> + put_device(&fwc->fwc_dev);
> + put_module:
> + module_put(owner);
> +}
> +EXPORT_SYMBOL(fw_coredumpm);
> +
> +static int __init fwcoredump_init(void)
> +{
> + return class_register(&fwc_class);
> +}
> +module_init(fwcoredump_init);
> +
> +static int fwc_free(struct device *dev, void *data)
> +{
> + struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> + flush_delayed_work(&fwc->del_wk);
> + return 0;
> +}
> +
> +static void __exit fwcoredump_exit(void)
> +{
> + class_for_each_device(&fwc_class, NULL, NULL, fwc_free);
> + class_unregister(&fwc_class);
> +}
> +module_exit(fwcoredump_exit);
> diff --git a/include/linux/fwcoredump.h b/include/linux/fwcoredump.h
> new file mode 100644
> index 000000000000..168f2d6abfc0
> --- /dev/null
> +++ b/include/linux/fwcoredump.h
> @@ -0,0 +1,35 @@
> +#ifndef __FWCOREDUMP_H
> +#define __FWCOREDUMP_H
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/vmalloc.h>
> +
> +#ifdef CONFIG_FW_COREDUMP
> +void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
> + gfp_t gfp);
> +
> +void fw_coredumpm(struct device *dev, struct module *owner,
> + const void *data, size_t datalen, gfp_t gfp,
> + ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> + const void *data, size_t datalen),
> + void (*free)(const void *data));
> +#else
> +static inline void fw_coredumpv(struct device *dev, const void *data,
> + size_t datalen, gfp_t gfp)
> +{
> + vfree(data);
> +}
> +
> +static inline void
> +fw_coredumpm(struct device *dev, struct module *owner,
> + const void *data, size_t datalen, gfp_t gfp,
> + ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> + const void *data, size_t datalen),
> + void (*free)(const void *data))
> +{
> + free(data);
> +}
> +#endif /* CONFIG_FW_COREDUMP */
> +
> +#endif /* __FWCOREDUMP_H */
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists