From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (146.0.238.70:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 25 Feb 2019 15:49:47 -0000 Received: from mail.kernel.org ([198.145.29.99]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1gyIVl-0000Yd-6t for speck@linutronix.de; Mon, 25 Feb 2019 16:49:46 +0100 Date: Mon, 25 Feb 2019 16:49:35 +0100 From: Greg KH Subject: [MODERATED] Re: [PATCH v6 31/43] MDSv6 Message-ID: <20190225154935.GA17057@kroah.com> References: <8d04705a73208de4bb4a4062bf3d977b5ee5c5f4.1551019522.git.ak@linux.intel.com> <20190225151935.GA19947@kroah.com> <20190225153411.GO16922@tassilo.jf.intel.com> MIME-Version: 1.0 In-Reply-To: <20190225153411.GO16922@tassilo.jf.intel.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Mon, Feb 25, 2019 at 07:34:11AM -0800, speck for Andi Kleen wrote: > On Mon, Feb 25, 2019 at 04:19:35PM +0100, speck for Greg KH wrote: > > On Sun, Feb 24, 2019 at 07:07:37AM -0800, speck for Andi Kleen wrote: > > > From: Andi Kleen > > > Subject: mds sweep: Clear cpu for usbmon intercepts > > > > > > usbmon touches user data in interrupts that otherwise don't > > > touch user data. Automatically schedule a clear cpu if > > > usbmon is called from an interrupt. > > > > I have written a long and very satisfying rant about this patch, that I > > than deleted, as it made me feel very better, but probably would not > > have helped anyone else out. > > > > In turn, I need you to properly justify this patch as these two tiny > > sentences, and this small patch make no sense to me at all. Please > > explain _WHY_ this is needed in this specific location. Before > > responding, I would strongly recommend reading up on exactly what usbmon > > is and who is allowed to use it. If after doing that, you still feel > > Right it's root only. > > But this is not about leaking data to the root monitoring user > (who can see the data anyways), but to unrelated processes > which are not root, but happen to be interrupted by the USB > interrupt. Then why are you messing around with the usbmon callback? It has nothing to do with anything here. By hooking it here, you now have 2 calls to this function on the USB urb callback path. The fact that a root process happens to be watching the USB data flowing through the system, or not, should have no affect on anything here, as the data flow is still the same (with the exception an extra copy in the irq could happen). Does multiple copys matter or not? I can't find anything in the documentation we have about this, am I missing it? > > this patch is needed (and it might be, I still can not tell for sure), > > Anything that touches user data in an interrupt needs to be marked > with the lazy approach. As I asked with the hcd change, what is "user data"? > I can write more on this instance. I nicely asked for that in the past but was ignored twice. Do I need to ask for it again in a non-nice manner? Without that information, this patchset is pretty impossible to review. > However I will probably not be able to write a detailed > description for each of the interrupt handlers changed because > there are just too many. Then how do you expect each subsystem / driver author to know if this is an acceptable change or not? How do you expect to educate driver authors to have them determine if they need to do this on their new drivers or not? Are you going to hand-audit each new driver that gets added to the kernel for forever? Without this type of information, this seems like a futile exercise. greg k-h