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  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:   Tue, 26 May 2020 10:43:02 +0800
From:   Weiyi Lu <weiyi.lu@...iatek.com>
To:     Matthias Brugger <matthias.bgg@...il.com>
CC:     Nicolas Boichat <drinkcat@...omium.org>,
        Stephen Boyd <sboyd@...nel.org>,
        James Liao <jamesjj.liao@...iatek.com>,
        <srv_heupstream@...iatek.com>, <linux-kernel@...r.kernel.org>,
        <stable@...r.kernel.org>, Fan Chen <fan.chen@...iatek.com>,
        <linux-mediatek@...ts.infradead.org>,
        Owen Chen <owen.chen@...iatek.com>,
        <linux-clk@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v1] clk: mediatek: assign the initial value to
 clk_init_data of mtk_mux

On Mon, 2020-05-25 at 11:08 +0200, Matthias Brugger wrote:
> 
> On 25/05/2020 08:41, Weiyi Lu wrote:
> > It'd be dangerous when struct clk_core have new memebers.
> > Add the missing initial value to clk_init_data.
> > 
> 
> Sorry I don't really understand this commit message, can please explain.
> In any case if this is a problem, then we probably we should fix it for all drivers.
> Apart from drivers/clk/mediatek/clk-cpumux.c
> 

Actually, we were looking into an android kernel patch "ANDROID: GKI:
clk: Add support for voltage voting" [1]

In this patch, there adds a new member struct clk_vdd_class	*vdd_class;
in struct clk_init_data and struct clk_core

And then in clk_register(...)
core->vdd_class = hw->init->vdd_class;

In many clock APIs, it will check the core->vdd_class to select the
correct control flow.
So, if we don't assign an initial value to clk_init_data of mtk_mux
clock type, something might go wrong. And assigning an initial value
might be the easiest and good way to avoid such problem if any new clock
support added in the future.

[1] https://android-review.googlesource.com/c/kernel/common/+/1278046

> It's a widely used pattern:
> $ git grep "struct clk_init_data init;"| wc -l
> 235
> 
> Regards,
> Matthias
> 
> > Fixes: a3ae549917f1 ("clk: mediatek: Add new clkmux register API")
> > Cc: <stable@...r.kernel.org>
> > Signed-off-by: Weiyi Lu <weiyi.lu@...iatek.com>
> > ---
> >  drivers/clk/mediatek/clk-mux.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > index 76f9cd0..14e127e 100644
> > --- a/drivers/clk/mediatek/clk-mux.c
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -160,7 +160,7 @@ struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
> >  				 spinlock_t *lock)
> >  {
> >  	struct mtk_clk_mux *clk_mux;
> > -	struct clk_init_data init;
> > +	struct clk_init_data init = {};
> >  	struct clk *clk;
> >  
> >  	clk_mux = kzalloc(sizeof(*clk_mux), GFP_KERNEL);
> > 

Powered by blists - more mailing lists