linux-kernel - Re: [PATCH 1/2] watchdog: bd70528: don't crash if WDG is confiured with BD71828

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]

Message-ID: <35ba5bcd48d4e027c0e5c839a856751519ee4dd1.camel@fi.rohmeurope.com>
Date:   Thu, 7 Jan 2021 18:15:36 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "linux@...ck-us.net" <linux@...ck-us.net>
CC:     "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
        "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        linux-power <linux-power@...rohmeurope.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>
Subject: Re: [PATCH 1/2] watchdog: bd70528: don't crash if WDG is confiured
 with BD71828

Thanks a lot for taking a careful look at this Guenter!

On Thu, 2021-01-07 at 07:12 -0800, Guenter Roeck wrote:
> On Thu, Jan 07, 2021 at 08:37:03AM +0200, Matti Vaittinen wrote:
> > If config for BD70528 watchdog is enabled when BD71828 or BD71815
> > are used the RTC module will issue call to BD70528 watchdog with
> > NULL data. Ignore this call and don't crash.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
> 
> I really think this should be handled in the calling code.
> Also, I am curious how this is supposed to work.
> 
> The code is called with
> 
> 	ret = bd70528_wdt_set(r->parent, new_state &
> BD70528_WDT_STATE_BIT,
>                               old_state);

My brainfart.
The bd70528_wdt_set is not called as it is protected in RTC by
has_rtc_timers flag.

I inserted this check in wrong function. The bd70528_wdt_lock()
is where we may hit the problem as it is not protected.
> 
> from bd70528_set_rtc_based_timers(). That same function subsequently
> calls bd70528_set_elapsed_tmr() with the same parameter, and that
> parameter is dereferenced in bd70528_set_elapsed_tmr() without
> checking.
> 
> Conceptually, it should not be necessary to determine at compile-time
> which of the chips is in the system. It should be posible to compile
> a single kernel which supports all chips.

I agree. The information whether WDT should be accessed should be
judged by dt-compatible. MFD has this knowledge and passes it to RTC.
So yes, RTC should omit the call if BD70528 is not used. Please ignore
these patches, I'll do changes to RTC driver :)


Best Regards
Matti