linux-kernel - Re: [PATCH] RISC-V: Check clint_time

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]

Date:   Sat, 26 Sep 2020 11:52:25 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@...ux-mips.org>
To:     Damien Le Moal <Damien.LeMoal@....com>
cc:     "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        Alistair Francis <Alistair.Francis@....com>,
        "palmerdabbelt@...gle.com" <palmerdabbelt@...gle.com>,
        Anup Patel <Anup.Patel@....com>,
        "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
        "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
        "palmer@...belt.com" <palmer@...belt.com>,
        "anup@...infault.org" <anup@...infault.org>,
        Atish Patra <Atish.Patra@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] RISC-V: Check clint_time_val before use

On Sat, 26 Sep 2020, Damien Le Moal wrote:

> > > With this applied in addition to your patch, it works.
> > > 
> > > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-
> > > clint.c
> > > index d17367dee02c..8dbec85979fd 100644
> > > --- a/drivers/clocksource/timer-clint.c
> > > +++ b/drivers/clocksource/timer-clint.c
> > > @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> > >  static unsigned int clint_timer_irq;
> > >  
> > >  #ifdef CONFIG_RISCV_M_MODE
> > > -u64 __iomem *clint_time_val;
> > > +u64 __iomem *clint_time_val = NULL;
> > >  #endif
> > 
> >  Hmm, BSS initialisation issue?
> 
> Not a static variable, so it is not in BSS, no ?

 Maybe it has a weird declaration elsewhere which messes up things (I 
haven't checked), but it looks to me like it does have static storage 
(rather than automatic or thread one), so if uninitialised it goes to BSS, 
and it is supposed to be all-zeros whether explicitly assigned a NULL 
value or not.  It does have external rather than internal linkage (as it 
would if it had the `static' keyword), but it does not matter.  Best check 
with `objdump'/`readelf'.

  Maciej