linux-kernel - Re: [PATCH v2] USB: move usb debugfs directory creation to the usb common core

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 5 Jun 2019 14:40:11 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Chunfeng Yun <chunfeng.yun@...iatek.com>
Cc:     Felipe Balbi <felipe.balbi@...ux.intel.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2] USB: move usb debugfs directory creation to the usb
 common core

On Wed, Jun 05, 2019 at 07:01:55PM +0800, Chunfeng Yun wrote:
> Hi Greg,
> On Wed, 2019-06-05 at 11:28 +0200, Greg Kroah-Hartman wrote:
> > The USB gadget subsystem wants to use the USB debugfs root directory, so
> > move it to the common "core" USB code so that it is properly initialized
> > and removed as needed.
> > 
> > In order to properly do this, we need to load the common code before the
> > usb core code, when everything is linked into the kernel, so reorder the
> > link order of the code.
> > 
> > Also as the usb common code has the possibility of the led trigger logic
> > to be merged into it, handle the build option properly by only having
> > one module init/exit function and have the common code initialize the
> > led trigger if needed.
> > 
> > Reported-by: From: Chunfeng Yun <chunfeng.yun@...iatek.com>
> > Cc: Felipe Balbi <felipe.balbi@...ux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > ---
> > 
> > Chunfeng, can you test this version to verify it works for you when
> > building the code into the kernel?
> > 
> > v2: handle led common code link error reported by kbuild
> >     handle subsys_initcall issue pointed out by Chunfeng
> > 
> >  drivers/usb/Makefile        |  3 +--
> >  drivers/usb/common/common.c | 21 +++++++++++++++++++++
> >  drivers/usb/common/common.h | 14 ++++++++++++++
> >  drivers/usb/common/led.c    |  9 +++------
> >  drivers/usb/core/usb.c      | 10 ++++------
> >  5 files changed, 43 insertions(+), 14 deletions(-)
> >  create mode 100644 drivers/usb/common/common.h
> > 
> > diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> > index 7d1b8c82b208..ecc2de1ffaae 100644
> > --- a/drivers/usb/Makefile
> > +++ b/drivers/usb/Makefile
> > @@ -5,6 +5,7 @@
> >  
> >  # Object files in subdirectories
> >  
> > +obj-$(CONFIG_USB_COMMON)	+= common/
> >  obj-$(CONFIG_USB)		+= core/
> >  obj-$(CONFIG_USB_SUPPORT)	+= phy/
> >  
> > @@ -60,8 +61,6 @@ obj-$(CONFIG_USB_CHIPIDEA)	+= chipidea/
> >  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
> >  obj-$(CONFIG_USB_GADGET)	+= gadget/
> >  
> > -obj-$(CONFIG_USB_COMMON)	+= common/
> > -
> >  obj-$(CONFIG_USBIP_CORE)	+= usbip/
> >  
> >  obj-$(CONFIG_TYPEC)		+= typec/
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 18f5dcf58b0d..84a4423aaddf 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -15,6 +15,8 @@
> >  #include <linux/usb/of.h>
> >  #include <linux/usb/otg.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/debugfs.h>
> > +#include "common.h"
> >  
> >  static const char *const ep_type_names[] = {
> >  	[USB_ENDPOINT_XFER_CONTROL] = "ctrl",
> > @@ -291,4 +293,23 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >  EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >  #endif
> >  
> > +struct dentry *usb_debug_root;
> > +EXPORT_SYMBOL_GPL(usb_debug_root);
> > +
> > +static int usb_common_init(void)
> > +{
> > +	usb_debug_root = debugfs_create_dir("usb", NULL);
> > +	ledtrig_usb_init();
> > +	return 0;
> > +}
> > +
> > +static void usb_common_exit(void)
> > +{
> > +	ledtrig_usb_exit();
> > +	debugfs_remove_recursive(usb_debug_root);
> > +}
> > +
> When enable CONFIG_LED_TRIGGER, there is a warning
> 
>  MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x68e300): Section mismatch in reference from
> the function usb_common_init() to the
> function .init.text:ledtrig_usb_init()
> The function usb_common_init() references
> the function __init ledtrig_usb_init().
> This is often because usb_common_init lacks a __init
> annotation or the annotation of ledtrig_usb_init is wrong.
> 
> WARNING: vmlinux.o(.text+0x68e318): Section mismatch in reference from
> the function usb_common_exit() to the
> function .exit.text:ledtrig_usb_exit()
> The function usb_common_exit() references a function in an exit section.
> Often the function ledtrig_usb_exit() has valid usage outside the exit
> section
> and the fix is to remove the __exit annotation of ledtrig_usb_exit.
> 
> seems need add __init and __exit for usb_common_init/exit

Yes, you are right, I'll go add those markings to those functions, good
catch.

greg k-h