linux-kernel - Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as _

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:   Mon, 13 Nov 2017 10:11:07 +0000
From:   Sudeep Holla <sudeep.holla@....com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Benjamin Gaignard <benjamin.gaignard@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as
 __init

On Sun, Nov 12, 2017 at 11:24:56PM +0100, Arnd Bergmann wrote:
> On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
> > On Mon, 6 Nov 2017, Arnd Bergmann wrote:
> >> The newly added function triggers a harmless Kbuild warning because
> >> of a missing annotation:
> >>
> >> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the function timer_of_exit() to the function .init.text:timer_clk_exit()
> >> The function timer_of_exit() references
> >> the function __init timer_clk_exit().
> >> This is often because timer_of_exit lacks a __init
> >> annotation or the annotation of timer_clk_exit is wrong.
> >>
> >> The function is only called from other __init functions, so it
> >> can safely be marked as __init as well.
> >
> > Hmm. I don't see any caller at all. From the intention of the patch I
> > assume this isn't designed for using from init functions, so we rather have
> > to remove the __init annotations from the called functions.
> >
> > Sudeep posted a patch which does that:
> >
> >  https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.holla@arm.com
> >
> > Though I rather would know whether this function is going to be used at
> > all and what the intention of this patch was.
> >
> > Benjamin????
> 
> My interpretation was that timer drivers are still supposed to be unregistered
> at module unload time, but that you might use the new timer_of_exit()
> in the failure path of whatever function calls timer_of_init() successfully
> when something fails in the next step.
> 
> Sudeep's interpretation also makes sense, I had not thought of that, but
> I now found the patch that adds a user in an init function:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html
> 
> It seems I guessed right and Sudeep guessed wrong (both by pure chance
> I admit).

Ah OK, I just went by name that it will be called by some exit/remove
function.

> Both patches solve the problem, Sudeep's version is a little
> more robust in case we ever add a caller in an __exit function (which I
> think is currently not allowed), while mine saves a little bit of memory
> and matches the current usage better.
>

Agreed, may be if we add users which is called from init functions, the
warning should disappear. Also as tglx suggested, we could rename if it's
just used from init function error/exit paths.

--
Regards,
Sudeep