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
| ||
|
Message-Id: <1191384586.8073.6.camel@concordia>
Date: Wed, 03 Oct 2007 14:09:46 +1000
From: Michael Ellerman <michael@...erman.id.au>
To: Tony Breeds <tony@...eyournoodle.com>
Cc: Linas Vepstas <linas@...tin.ibm.com>, linuxppc-dev@...abs.org,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org
Subject: Re: 2.6.23-rc7-mm1 -- powerpc rtas panic
On Wed, 2007-10-03 at 11:19 +1000, Tony Breeds wrote:
> On Wed, Oct 03, 2007 at 10:30:16AM +1000, Michael Ellerman wrote:
>
> > I realise it'll make the patch bigger, but this doesn't seem like a
> > particularly good name for the variable anymore.
>
> Sure, what about?
Better .. but .. :D
> diff --git a/arch/powerpc/platforms/pseries/rtasd.c b/arch/powerpc/platforms/pseries/rtasd.c
> index 30925d2..73401c8 100644
> --- a/arch/powerpc/platforms/pseries/rtasd.c
> +++ b/arch/powerpc/platforms/pseries/rtasd.c
> @@ -54,8 +54,9 @@ static unsigned int rtas_event_scan_rate;
> static int full_rtas_msgs = 0;
>
> /* Stop logging to nvram after first fatal error */
> -static int no_more_logging;
> -
> +static int logging_enabled; /* Until we initialize everything,
> + * make sure we don't try logging
> + * anything */
Until we initialise what exactly?
> static int error_log_cnt;
>
> /*
> @@ -217,7 +218,7 @@ void pSeries_log_error(char *buf, unsigned int err_type, int fatal)
> }
>
> /* Write error to NVRAM */
> - if (!no_more_logging && !(err_type & ERR_FLAG_BOOT))
> + if (logging_enabled && !(err_type & ERR_FLAG_BOOT))
> nvram_write_error_log(buf, len, err_type, error_log_cnt);
>
> /*
> @@ -229,8 +230,8 @@ void pSeries_log_error(char *buf, unsigned int err_type, int fatal)
> printk_log_rtas(buf, len);
>
> /* Check to see if we need to or have stopped logging */
> - if (fatal || no_more_logging) {
> - no_more_logging = 1;
> + if (fatal || !logging_enabled) {
> + logging_enabled = 0;
> spin_unlock_irqrestore(&rtasd_log_lock, s);
> return;
> }
Hmmm, this routine has 4 separate lock-dropping exit paths ..
> @@ -302,7 +303,7 @@ static ssize_t rtas_log_read(struct file * file, char __user * buf,
>
> spin_lock_irqsave(&rtasd_log_lock, s);
> /* if it's 0, then we know we got the last one (the one in NVRAM) */
> - if (rtas_log_size == 0 && !no_more_logging)
> + if (rtas_log_size == 0 && logging_enabled)
> nvram_clear_error_log();
> spin_unlock_irqrestore(&rtasd_log_lock, s);
>
> @@ -414,6 +415,8 @@ static int rtasd(void *unused)
> memset(logdata, 0, rtas_error_log_max);
> rc = nvram_read_error_log(logdata, rtas_error_log_max,
> &err_type, &error_log_cnt);
> + /* We can use rtas_log_buf now */
> + logging_enabled = 1;
>
> if (!rc) {
> if (err_type != ERR_FLAG_ALREADY_LOGGED) {
What exactly happens that allows us to do logging? I don't see any
ordering between anything else and the setting of the flag, and AFAICT
we're not inside a spinlock or anything here.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)
Powered by blists - more mailing lists