linux-kernel - Re: [PATCH v5 1/4] of: change overlay apply input data from unflattened to FDT

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:   Fri, 2 Mar 2018 10:20:32 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Frank Rowand <frowand.list@...il.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
        Pantelis Antoniou <panto@...oniou-consulting.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
Subject: Re: [PATCH v5 1/4] of: change overlay apply input data from
 unflattened to FDT

Hi Frank,

On Fri, Mar 2, 2018 at 2:51 AM,  <frowand.list@...il.com> wrote:
> There are still some functions in unittest.c that should be tagged
> __init due to changes in this patch, but modpost is not warning of
> them and they are not a risk because they are only called from
> __init functions.  A sweep of unittest.c for functions that
> should be tagged __init is on the todo list.

If modpost doesn't warn, that merely means your compiler decided to
inline all functions with wrong annotations, hiding the problem.
Other (versions of) compilers may behave differently, so we do want
to get this right.

With my trusty gcc-4.1.2:

    WARNING: vmlinux.o(.text+0x342dd4): Section mismatch in reference
from the function of_unittest_apply_revert_overlay_check() to the
function .init.text:of_unittest_apply_overlay()
    The function of_unittest_apply_revert_overlay_check() references
    the function __init of_unittest_apply_overlay().
    This is often because of_unittest_apply_revert_overlay_check lacks a __init
    annotation or the annotation of of_unittest_apply_overlay is wrong.

To fix the above:

-static int of_unittest_apply_revert_overlay_check(int overlay_nr,
+static int __init of_unittest_apply_revert_overlay_check(int overlay_nr,
-static void of_unittest_overlay_5(void)
+static void __init of_unittest_overlay_5(void)
-static void of_unittest_overlay_11(void)
+static void __init of_unittest_overlay_11(void)

> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c

> @@ -2290,18 +2275,29 @@ static __init void of_unittest_overlay_high_level(void)
>                 __of_attach_node_sysfs(np);
>
>         if (of_symbols) {
> +               struct property *new_prop;
>                 for_each_property_of_node(overlay_base_symbols, prop) {

drivers/of/unittest.c: In function ‘of_unittest_overlay_high_level’:
drivers/of/unittest.c:2193: warning: ‘overlay_base_symbols’ may be
used uninitialized in this function

This isn't a new warning, so I guess I never reported it before because I
thought it was a false positive (misguided by the "if (of_symbols)" test?).

However, now I believe it is not, and an uninitialized pointer will be
dereferenced if of_root has a __symbols__ node, but overlay_base_root hasn't.

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