linux-kernel - Re: [RFC v2] device coredump: add new device coredump class

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:	Sun, 7 Sep 2014 10:42:45 +0300
From:	Arik Nemtsov <arik@...ery.com>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
	Luciano Coelho <luca@...lho.fi>, Kalle Valo <kvalo@...rom.com>,
	dri-devel@...ts.freedesktop.org,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
Subject: Re: [RFC v2] device coredump: add new device coredump class

On Fri, Sep 5, 2014 at 11:50 AM, Johannes Berg
<johannes@...solutions.net> wrote:
> From: Johannes Berg <johannes.berg@...el.com>
>
> Many devices run firmware and/or complex hardware, and most of that
> can have bugs. When it misbehaves, however, it is often much harder
> to debug than software running on the host.
>

Very nice concept IMO :)

[....]
> +void dev_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 devcd_count = ATOMIC_INIT(0);
> +       struct devcd_entry *devcd;
> +       struct device *existing;
> +
> +       existing = class_find_device(&devcd_class, NULL, dev,
> +                                    devcd_match_failing);
> +       if (existing) {
> +               put_device(existing);
> +               return;
> +       }
> +
> +       if (!try_module_get(owner))
> +               return;
> +
> +       devcd = kzalloc(sizeof(*devcd), gfp);
> +       if (!devcd)
> +               goto put_module;
> +
> +       devcd->owner = owner;
> +       devcd->data = data;
> +       devcd->datalen = datalen;
> +       devcd->read = read;
> +       devcd->free = free;
> +       devcd->failing_dev = get_device(dev);
> +
> +       device_initialize(&devcd->devcd_dev);
> +
> +       dev_set_name(&devcd->devcd_dev, "devcd%d",
> +                    atomic_inc_return(&devcd_count));
> +       devcd->devcd_dev.class = &devcd_class;
> +
> +       if (device_add(&devcd->devcd_dev))
> +               goto put_device;
> +
> +       if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
> +                             "failing_device"))
> +               /* nothing - symlink will be missing */;
> +
> +       if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> +                             "dev_coredump"))
> +               /* nothing - symlink will be missing */;

Aren't you confusing the compiler here and above, doing the
INIT_DELAYED_WORK only when link creation failed? I would have thought
some braces are necessary..

> +
> +       INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> +       schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> +
> +       return;
> + put_device:
> +       put_device(&devcd->devcd_dev);
> + put_module:
> +       module_put(owner);
> +}
> +EXPORT_SYMBOL(dev_coredumpm);
--
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/