linux-kernel - Re: [PATCH v2] clocksource/drivers/integrator-ap: add missing of_node

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] [day] [month] [year] [list]

Message-ID: <CAEExFWuUU36hr3Xagy62GiJ-GeOytg7TJm+B0gpjW-W25hhc7w@mail.gmail.com>
Date:   Sun, 25 Nov 2018 17:48:45 +0800
From:   Frank Lee <tiny.windzz@...il.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     tglx@...utronix.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] clocksource/drivers/integrator-ap: add missing of_node_put()

On Sun, Nov 25, 2018 at 5:43 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> On 25/11/2018 06:00, Yangtao Li wrote:
> > of_find_node_by_path() acquires a reference to the node
> > returned by it and that reference needs to be dropped by its caller.
> > integrator_ap_timer_init_of() doesn't do that.The pri_node and the
> > sec_node are used as an identifier to compare against the current node,
> > we can directly drop the refcount after getting the node from path as
> > it is not used aspointer. In addition, a single variable is needed,
> > so fix it.
>
> I can apply the patch after adding a comment in the code below or
> alternatively you can resend a V3 after adding a comment yourself.
>
> What is your preference?
Hi Daniel:

You can add a comment in the code below,then apply the patch.

Thanks,
Yangtao
>
> > Signed-off-by: Yangtao Li <tiny.windzz@...il.com>
> > ---
> > Changes in v2:
> > -update changeelog
> > -simplify fix
> > -change two variable to one
> > ---
> >  drivers/clocksource/timer-integrator-ap.c | 22 +++++++++++++---------
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c
> > index 76e526f58620..bbd0977d579a 100644
> > --- a/drivers/clocksource/timer-integrator-ap.c
> > +++ b/drivers/clocksource/timer-integrator-ap.c
> > @@ -181,8 +181,7 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
> >       int irq;
> >       struct clk *clk;
> >       unsigned long rate;
> > -     struct device_node *pri_node;
> > -     struct device_node *sec_node;
> > +     struct device_node *alias_node;
> >
> >       base = of_io_request_and_map(node, 0, "integrator-timer");
> >       if (IS_ERR(base))
> > @@ -204,7 +203,14 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
> >               return err;
> >       }
> >
> > -     pri_node = of_find_node_by_path(path);
> > +     alias_node = of_find_node_by_path(path);
> > +
> > +     /* Drop the refcount of node */
> > +     of_node_put(alias_node);
> > +
> > +     if (node == alias_node)
> > +             /* The primary timer lacks IRQ, use as clocksource */
> > +             return integrator_clocksource_init(rate, base);
> >
> >       err = of_property_read_string(of_aliases,
> >                               "arm,timer-secondary", &path);
> > @@ -213,14 +219,12 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
> >               return err;
> >       }
> >
> > +     alias_node = of_find_node_by_path(path);
> >
> > -     sec_node = of_find_node_by_path(path);
> > -
> > -     if (node == pri_node)
> > -             /* The primary timer lacks IRQ, use as clocksource */
> > -             return integrator_clocksource_init(rate, base);
> > +     /* Drop the refcount of node */
> > +     of_node_put(alias_node);
> >
> > -     if (node == sec_node) {
> > +     if (node == alias_node) {
> >               /* The secondary timer will drive the clock event */
> >               irq = irq_of_parse_and_map(node, 0);
> >               return integrator_clockevent_init(rate, base, irq);
> >
>
>
> --
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>