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
| ||
|
Message-ID: <Pine.LNX.4.44L0.1605241007160.1745-100000@iolanthe.rowland.org> Date: Tue, 24 May 2016 10:23:07 -0400 (EDT) From: Alan Stern <stern@...land.harvard.edu> To: "Du, Changbin" <changbin.du@...el.com> cc: gregkh@...uxfoundation.org, <mathias.nyman@...ux.intel.com>, <stefan.koch10@...il.com>, <hkallweit1@...il.com>, <sergei.shtylyov@...entembedded.com>, <jonas.hesselmann@...mail.de>, <mingo@...nel.org>, <paulmck@...ux.vnet.ibm.com>, <akpm@...ux-foundation.org>, <tglx@...utronix.de>, <keescook@...omium.org>, <dan.j.williams@...el.com>, <aryabinin@...tuozzo.com>, <tj@...nel.org>, <dave@...olabs.net>, <nikolay@...ulusnetworks.com>, <dvyukov@...gle.com>, <adrien+dev@...ischi.me>, USB list <linux-usb@...r.kernel.org>, Kernel development list <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] usb: core: add debugobjects support for urb object On Tue, 24 May 2016 changbin.du@...el.com wrote: > From: "Du, Changbin" <changbin.du@...el.com> > > Add debugobject support to track the life time of struct urb. > This feature help us detect violation of urb operations by > generating a warning message from debugobject core. I'm pretty sure the USB core already does this tracking. If it doesn't, why not make the appropriate changes instead of adding a whole new infrastructure? > And we fix > the possible issues at runtime to avoid oops if we can. This is questionable. Under what conditions do you think you can fix up a possible issue? > I have done some tests with some class drivers, no violation > found in them which is good. Expect this feature can be used > for debugging future problems. > > Signed-off-by: Du, Changbin <changbin.du@...el.com> > --- > drivers/usb/core/hcd.c | 1 + > drivers/usb/core/urb.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++-- > include/linux/usb.h | 8 ++++ > lib/Kconfig.debug | 8 ++++ > 4 files changed, 130 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index 34b837a..a8ea128 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -1750,6 +1750,7 @@ static void __usb_hcd_giveback_urb(struct urb *urb) > /* pass ownership to the completion handler */ > urb->status = status; > > + debug_urb_deactivate(urb); > /* > * We disable local IRQs here avoid possible deadlock because > * drivers may call spin_lock() to hold lock which might be > diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c > index c601e25..0d1eccb 100644 > --- a/drivers/usb/core/urb.c > +++ b/drivers/usb/core/urb.c > @@ -10,6 +10,100 @@ > > #define to_urb(d) container_of(d, struct urb, kref) > > +#ifdef CONFIG_DEBUG_OBJECTS_URB > +static struct debug_obj_descr urb_debug_descr; > + > +static void *urb_debug_hint(void *addr) > +{ > + return ((struct urb *) addr)->complete; > +} > + > +/* > + * fixup_init is called when: > + * - an active object is initialized > + */ > +static bool urb_fixup_init(void *addr, enum debug_obj_state state) > +{ > + struct urb *urb = addr; > + > + switch (state) { > + case ODEBUG_STATE_ACTIVE: > + usb_kill_urb(urb); > + debug_object_init(urb, &urb_debug_descr); > + return true; > + default: > + return false; > + } > +} Is it guaranteed that this routine and the other new ones can be called only in process context? I don't think so -- but usb_kill_urb will oops if it is called with interrupts disabled. > @@ -539,10 +635,23 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) > } > } > > - return usb_hcd_submit_urb(urb, mem_flags); > + ret = debug_urb_activate(urb); > + if (ret) > + return ret; > + ret = usb_hcd_submit_urb(urb, mem_flags); > + if (ret) > + debug_urb_deactivate(urb); > + > + return ret; > } > EXPORT_SYMBOL_GPL(usb_submit_urb); > > +static inline int __usb_unlink_urb(struct urb *urb, int status) > +{ > + debug_urb_deactivate(urb); > + return usb_hcd_unlink_urb(urb, status); > +} This is a mistake. The URB is still active until it is given back to the completion routine. Alan Stern
Powered by blists - more mailing lists