AW: AW: KASAN: use-after-free Read in usbhid_power

From: "Schmid, Carsten" <Carsten_Schmid@mentor.com>
To: Hans de Goede <hdegoede@redhat.com>,
	Greg KH <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Andrey Konovalov <andreyknvl@google.com>,
	Oliver Neukum <oneukum@suse.com>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	syzbot <syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com>,
	USB list <linux-usb@vger.kernel.org>,
	Hillf Danton <hdanton@sina.com>
Subject: AW: AW: KASAN: use-after-free Read in usbhid_power
Date: Fri, 9 Aug 2019 10:47:50 +0000	[thread overview]
Message-ID: <86ef050c477841a6951fd984b38c9f04@SVR-IES-MBX-03.mgc.mentorg.com> (raw)
In-Reply-To: <d7f1f3dc-03b3-34b8-657b-13c7b91ee361@redhat.com>

> 
> We are talking memory-mapped io here, so it cannot just be "re-used", it
> is wat it is. I guess the PCI BAR could be released and then the physical
> address the resource was at could be re-used for another piece of MMIo,
> but AFAIK outside of PI=CI hotplug we never release BARs.
> 
> Maybe we need to ref-count resources and have the aprent free only be
> a deref and not release the resource until the child resource also
> is free-ed doing another deref?
> 
> I must say that to me it sometimes just seems like always allowing unbind
> is a bad idea. Another example of this is things like virtio, where
> we can have a filesystem based on virtio-block, but the virtio interface
> between the hypervisor and the guest-kernel is a PCI-device and in theory
> the user could unbind the virtio driver from that PCI-device, after which
> the whole house comes crashing down.
> 
> I also know that the extcon framework in its current incarnaton
> does not deal with unbind properly...
> 
> Maybe it is time that we allow drivers to block unbind instead of
> trying to support unbind in really complex situations where normal
> use-cases will never need it ?
> 
> I do realize unbind is very useful for driver developent without
> rebooting.
> 

Hey, i did not want to trigger an eartquake in the basement of the kernel ;-)
My intention was to prevent some crashes, and help developers to find their bugs.
I think my patch exactly does this.

> 1) make resources refcounted, have child resources take a ref on the parent
> 2) Disallow unbind on devices with bound child-devices?
> 

Exactly what i was thinking of in first attempts.
But i fear that would break even more use cases.

Hans, directly regarding the driver:
The problem i see is that the xhci_intel_unregister_pdev which is added
as an action with devm_add_action_or_reset() is called late by the framework,
later than the usb_hcd_pci_remove() in xhci_pci_remove.
Is there any chance to trigger this before?
This is what Greg meant with "right order".

Anyway, i really appreciate these discussions, thanks for all
your patience.

Best Regards
Carsten