linux-kernel - Re: [xen] double fault: 0000 [#1] PREEMPT SMP DEBUG

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAPM=9twEehVg4JNqVrvEgCQ6R_0Saae0e9Gj2EsQSFviSywzEQ@mail.gmail.com>
Date:	Fri, 11 Oct 2013 13:55:21 +1000
From:	Dave Airlie <airlied@...il.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Dave Airlie <airlied@...ux.ie>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	xen-devel@...ts.xenproject.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [xen] double fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC

On Thu, Oct 10, 2013 at 8:53 PM, Russell King - ARM Linux
<linux@....linux.org.uk> wrote:
> On Thu, Oct 10, 2013 at 10:19:20AM +0100, Russell King - ARM Linux wrote:
>> On Thu, Oct 10, 2013 at 03:23:45AM +0100, Dave Airlie wrote:
>> >
>> > > I think David Arlie also needs a quiet talking to about how to use the
>> > > device model:
>> > >
>> > > int drm_sysfs_device_add(struct drm_minor *minor)
>> > > {
>> > >         minor->kdev.release = drm_sysfs_device_release;
>> > > ...
>> > >         err = device_register(&minor->kdev);
>> > > }
>> > >
>> > > static void drm_sysfs_device_release(struct device *dev)
>> > > {
>> > >         memset(dev, 0, sizeof(struct device));
>> > >         return;
>> > > }
>> > >
>> > > Since when has that been acceptable in a release function?
>> >
>> > Well the commit that added it had a reason that seems to cover some other
>> > device model abuses, so maybe someone who actually understands the device
>> > model (all 2 people) can review usage.
>>
>> Please - there's more than two people who understand this stuff.
>>
>> You appear to manage to understand the refcounting principle for things
>> like the DRM framebuffers, DRM buffer objects etc, and the driver model
>> (and kobjects in general) are no different.
>>
>> I've just been reading the code around here little more, and hit the usb
>> implementation.  I don't see how USB drivers get cleaned up when they're
>> disconnected from the USB bus.  I see drm_unplug_dev() which gets called
>> on a USB disconnection (from udl/udl_drv.c) which effectively makes the
>> device unavailable, but I don't see anything which frees anything.  I see
>> nothing calling drm_put_dev() here.  How does the drm_device in this case
>> get freed?
>
> Don't worry about the above - I've worked out how USB works in that regard.
> However, I can't say that I like the feel of the code in drm_unplug_dev()
> and drm_put_dev() - if these two can run simultaneously in two threads of
> execution, there's the chance that things will go awry.  I don't think
> that can happen due to the way the unplugged-ness is dealt with, but it
> doesn't "feel" safe.
>
> I think something like the below may address the issue - I've only build
> tested this so far.
>
> We have another case where DRM does the wrong thing here too - a similar
> thing goes on with connectors as well.  I've not yet looked into this,
> but I'll take a look later today.
>

Damn gmail screwed up my reply-all,

anyhoo I get the feeling I'd rather do this like fbdev does and stop using
an embedded kdev for this if I can. The lifetime of the minor and the sysfs
objects aren't necessarily that tied together esp with hot unplug of
USB devices.

e.g. when a USB device goes away I don't want the sysfs files to
remain hanging around,
or the userspace device nodes, however I can't remove all the internal
driver state until userspace finally closes all open device
files,

I think this is a bit of a bad misdesign of the whole device api that
sysfs exposure and device exposure is tied
to objects you are encourgaged to embed.

I think maybe I'll just copy fbdev and do a separate device_create
with a nonembedded dev.

Dave.
--
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/