linux-kernel - Re: ERROR: modpost: "__delay" [drivers/net/mdio/mdio-cavium.ko] undefined!

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: <CAMuHMdWiPjuf47=SBjNrQNjf4QiwDf6t0kkx_5BdPVoa22zxmw@mail.gmail.com>
Date:   Mon, 31 May 2021 17:45:10 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     David Daney <david.daney@...ium.com>,
        kernel test robot <lkp@...el.com>,
        Andrew Lunn <andrew@...n.ch>, kbuild-all@...ts.01.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Rich Felker <dalias@...c.org>
Subject: Re: ERROR: modpost: "__delay" [drivers/net/mdio/mdio-cavium.ko] undefined!

Hi Randy,

On Mon, May 31, 2021 at 5:12 PM Randy Dunlap <rdunlap@...radead.org> wrote:
> On 5/31/21 12:32 AM, Geert Uytterhoeven wrote:
> > CC David (original author, asked by driver location change)
> >
> > On Mon, May 31, 2021 at 9:29 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> >>
> >> On Mon, May 31, 2021 at 2:05 AM Randy Dunlap <rdunlap@...radead.org> wrote:
> >>> On 5/29/21 4:25 PM, kernel test robot wrote:
> >>>> First bad commit (maybe != root cause):
> >>>>
> >>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >>>> head:   df8c66c4cfb91f2372d138b9b714f6df6f506966
> >>>> commit: a9770eac511ad82390b9f4a3c1728e078c387ac7 net: mdio: Move MDIO drivers into a new subdirectory
> >>>> date:   9 months ago
> >>>> config: sh-allmodconfig (attached as .config)
> >>>> compiler: sh4-linux-gcc (GCC) 9.3.0
> >>>> reproduce (this is a W=1 build):
> >>>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >>>>         chmod +x ~/bin/make.cross
> >>>>         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9770eac511ad82390b9f4a3c1728e078c387ac7
> >>>>         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >>>>         git fetch --no-tags linus master
> >>>>         git checkout a9770eac511ad82390b9f4a3c1728e078c387ac7
> >>>>         # save the attached .config to linux build tree
> >>>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh
> >>>>
> >>>> If you fix the issue, kindly add following tag as appropriate
> >>>> Reported-by: kernel test robot <lkp@...el.com>
> >>>>
> >>>> All errors (new ones prefixed by >>, old ones prefixed by <<):
> >>>>
> >>>>>> ERROR: modpost: "__delay" [drivers/net/mdio/mdio-cavium.ko] undefined!
> >>>
> >>> Just a comment here.  kernel test robot has reported this issue
> >>> 5 times in 2021 that I know of -- and I could have missed some.
> >>>
> >>> I see that Geert recently (June 2020) reverted the
> >>> EXPORT_SYMBOL(__delay) in arch/sh/lib/delay.c, with this comment:
> >>>
> >>>     __delay() is an internal implementation detail on several architectures.
> >>>     Drivers should not call __delay() directly, as it has non-standardized
> >>>     semantics, or may not even exist.
> >>>     Hence there is no need to export __delay() to modules.
> >>>
> >>>     See also include/asm-generic/delay.h:
> >>>
> >>>         /* Undefined functions to get compile-time errors */
> >>>         ...
> >>>         extern void __delay(unsigned long loops);
> >>>
> >>> However, s/several architectures/all but one architecture: SH/.
> >>> All architectures except for SH provide either an exported function,
> >>> an inline function, or a macro for __delay(). Yeah, they probably
> >>> don't all do the same delay.
> >>
> >> Hence it must not be used by drivers, as it might give the false assumption
> >> of working everywhere.  While drivers/net/mdio/mdio-cavium is
> >> platform-specific, code might be copied in a new driver, less restricted
> >> to a specific platform.
>
> Geert, should all (15) of the other arch EXPORT_SYMBOL(__delay); exports
> be removed?  (in theory?  I'm not planning to remove them.)

It depends.  If they are internal implementation details of an
architecture's mdelay() or udelay() function (i.e. the latter are
static inline functions that may call into out-of-line __delay()
functions), then they should be kept.

I haven't checked all of them, but e.g. on arm64, mdelay() and udelay()
don't call into __delay, so IMHO its export should be removed.

Generic drivers should not use __delay() with a hardcoded value, as
its semantics are not defined (cfr. the undefined function comment
in asm-generic).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds