[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFmMkTG342Thq+H-om+3mCog5Yh3ZzqO_HYnR-Jv-b1utojk0g@mail.gmail.com>
Date: Tue, 4 Aug 2020 12:05:25 -0300
From: Daniel Gutson <daniel@...ypsium.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Derek Kiernan <derek.kiernan@...inx.com>,
Tudor Ambarus <tudor.ambarus@...rochip.com>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Arnd Bergmann <arnd@...db.de>,
Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Richard Hughes <hughsient@...il.com>,
Alex Bazhaniuk <alex@...ypsium.com>
Subject: Re: [PATCH] Platform lockdown information in SYSFS
On Tue, Aug 4, 2020 at 11:43 AM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Tue, Aug 04, 2020 at 11:37:02AM -0300, Daniel Gutson wrote:
> > On Tue, Aug 4, 2020 at 11:23 AM Greg Kroah-Hartman <
> > gregkh@...uxfoundation.org> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 10:50:13AM -0300, Daniel Gutson wrote:
> > > > On Tue, Aug 4, 2020 at 3:41 AM Greg Kroah-Hartman
> > > > <gregkh@...uxfoundation.org> wrote:
> > > > >
> > > > > On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > > > > > > Think of this as an input device. You don't put the random
> > input
> > > > > > > > > attributes all in one place, you create a new device that
> > represents the
> > > > > > > > > input interface and register that.
> > > > > >
> > > > > > I'm having trouble with this. What's the dev_t for the child
> > devices?
> > > > > > I'm doing
> > > > > > child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > > > > > NULL, "child");
> > > > > > pdev is the pci_device (intel-spi-pci)
> > > > > > dmesg shows
> > > > > >
> > > > > > sysfs: cannot create duplicate filename '/class/my-class'
> > > > > > (call trace)
> > > > > > kobject_add_internal failed for my-class with -EEXIST, don't try
> > > > > > to register things with the same name in the same directory.
> > > > >
> > > > > Without seeing all of your code, I can't tell you what you are doing
> > > > > wrong, but the kernel should be giving you a huge hint here...
> > > > >
> > > > > Don't create duplicate names in the same subdirectory.
> > > >
> > > > I'm not doing that. One of my questions is if MKDEV(0, 0) is valid for
> > > > create_device, which I inferred so from the documentation.
> > >
> > > Yes it is, but that's not the error given to you :)
> > >
> > > Many in-kernel users call device_create() with MKDEV(0, 0)
> > >
> > > > Here is the listing
> > >
> > > <snip>
> > >
> > > It's not in any format to read, please never strip leading whitespace,
> > > it hurts my brain...
> >
> > (trying again)
> > Also, this is in pastebin: https://pastebin.com/8Ye9eUm5
> >
> > #include <linux/kobject.h>
> > #include <linux/sysfs.h>
> > #include <linux/module.h>
> > #include <linux/init.h>
> > #include <linux/list.h>
> > #include <linux/slab.h>
> > #include <linux/device.h>
> > #include <linux/pci.h>
> >
> > static ssize_t howareyou_show(struct class *class, struct class_attribute
> > *attr,
> > char *buf)
> > {
> > return sprintf(buf, "%s\n", "How are you?");
> > }
> > static CLASS_ATTR_RO(howareyou);
>
> These are rare, as they are "global" for a class, are you sure you want
> that?
I'm no longer using class attributes, this is from my previous
experiment. Sorry for the confusion.
In the "real" code I'll use DEVICE_ATTR_RO.
>
> >
> > static struct class my_class = {
> > .name = "my-class",
> > .owner = THIS_MODULE,
> > };
> >
> > struct device* child_device;
> >
> > static int mypci_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> > {
> > int ret;
> >
> > ret = pcim_enable_device(pdev);
> > if (ret)
> > return ret;
> >
> > ret = class_register(&my_class);
> > if (ret < 0)
> > return ret;
> >
> >
> > pr_info("DFG: Recognized. DID: %lx\n", (unsigned long
> > int)id->driver_data);
> > pr_info("DFG: device DID: %lx\n", (unsigned long int)pdev->device);
> >
> > ret = class_create_file(&my_class, &class_attr_howareyou);
> > if (ret != 0) {
> > pr_err("DFG class create file error: %d\n", ret);
> > class_unregister(&my_class);
> > return ret;
> > }
> >
> > child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > NULL, "child");
> > if (child_device == NULL) {
> > pr_err("DFG error child device NULL");
> > }
> >
> > return ret;
> > }
> >
>
>
> Looks sane, what does your kernel log say when you load this?
first insmod, OK.
rmmod, OK.
Second insmod:
[ 4149.389133] sysfs: cannot create duplicate filename
'/devices/pci0000:00/0000:00:1f.0/my-class'
[ 4149.389135] CPU: 2 PID: 8900 Comm: insmod Tainted: G OE
5.8.0+ #1
[ 4149.389136] Hardware name: xxxxx
[ 4149.389136] Call Trace:
[ 4149.389141] dump_stack+0x74/0x9a
[ 4149.389143] sysfs_warn_dup.cold+0x17/0x35
[ 4149.389144] sysfs_create_dir_ns+0xb8/0xd0
[ 4149.389146] kobject_add_internal+0xbd/0x2b0
[ 4149.389147] kobject_add+0x7e/0xb0
[ 4149.389149] ? _cond_resched+0x19/0x30
[ 4149.389151] ? kmem_cache_alloc_trace+0x16c/0x240
[ 4149.389154] get_device_parent.isra.0+0x179/0x1b0
[ 4149.389155] device_add+0xed/0x830
[ 4149.389156] device_create_groups_vargs+0xd4/0xf0
[ 4149.389157] device_create+0x49/0x60
[ 4149.389159] mypci_probe.cold+0x79/0x9a [firmware_security_class]
[ 4149.389160] local_pci_probe+0x48/0x80
[ 4149.389161] pci_device_probe+0x10f/0x1c0
[ 4149.389162] really_probe+0x159/0x3e0
[ 4149.389163] driver_probe_device+0xe9/0x160
[ 4149.389164] device_driver_attach+0x5d/0x70
[ 4149.389165] __driver_attach+0x8f/0x150
[ 4149.389166] ? device_driver_attach+0x70/0x70
[ 4149.389167] bus_for_each_dev+0x7e/0xc0
[ 4149.389168] driver_attach+0x1e/0x20
[ 4149.389168] bus_add_driver+0x152/0x1f0
[ 4149.389169] driver_register+0x74/0xd0
[ 4149.389170] ? 0xffffffffc0cb8000
[ 4149.389171] __pci_register_driver+0x54/0x60
[ 4149.389172] my_pci_driver_init+0x23/0x1000 [firmware_security_class]
[ 4149.389174] do_one_initcall+0x4a/0x200
[ 4149.389175] ? _cond_resched+0x19/0x30
[ 4149.389176] ? kmem_cache_alloc_trace+0x16c/0x240
[ 4149.389177] do_init_module+0x62/0x240
[ 4149.389178] load_module+0x291f/0x2b90
[ 4149.389180] __do_sys_finit_module+0xbe/0x120
[ 4149.389181] ? __do_sys_finit_module+0xbe/0x120
[ 4149.389182] __x64_sys_finit_module+0x1a/0x20
[ 4149.389183] do_syscall_64+0x52/0xc0
[ 4149.389184] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 4149.389185] RIP: 0033:0x7fbdc4b8c70d
[ 4149.389186] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e
fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 53 f7 0c 00 f7 d8 64 89
01 48
[ 4149.389187] RSP: 002b:00007ffd7f2b7d88 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 4149.389188] RAX: ffffffffffffffda RBX: 000055b301ef4770 RCX: 00007fbdc4b8c70d
[ 4149.389188] RDX: 0000000000000000 RSI: 000055b301b4d358 RDI: 0000000000000003
[ 4149.389188] RBP: 0000000000000000 R08: 0000000000000000 R09: 00007fbdc4c60260
[ 4149.389189] R10: 0000000000000003 R11: 0000000000000246 R12: 000055b301b4d358
[ 4149.389189] R13: 0000000000000000 R14: 000055b301ef7180 R15: 0000000000000000
[ 4149.389190] kobject_add_internal failed for my-class with -EEXIST,
don't try to register things with the same name in the same directory.
>
> thanks,
>
> greg k-h
--
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium
Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport
Powered by blists - more mailing lists