linux-kernel - Re: [PATCH] AT91 Serial: update the powersave handler to match serial core

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: <20080919164757.2ee78a09@hskinnemo-gx745.norway.atmel.com>
Date:	Fri, 19 Sep 2008 16:47:57 +0200
From:	Haavard Skinnemoen <haavard.skinnemoen@...el.com>
To:	Anti Sullin <anti.sullin@...ecdesign.ee>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] AT91 Serial: update the powersave handler to match
 serial core

Haavard Skinnemoen <haavard.skinnemoen@...el.com> wrote:
> > > Are both of these checks necessary? It doesn't look like
> > > atmel_console_putchar() can be called from anywhere else than
> > > atmel_console_write().  
> > Actually, not so much. I added them for protection so that we wouldn't run into
> > a deadlock loop if something changes again. Those while() loops make me cautious.
> > And I didn't have the time to look up if the suspend code is thread-safe without this.  
> 
> I don't think it's much safer with that extra check if suspend can be
> called in parallel with this. In that case, the only way to make
> _absolutely_ sure would be to move the test inside the while loops.
> 
> And I only think this actually matters if console suspend is disabled,
> which should probably never happen on a production system. So I'm
> wondering if we really need any of those checks.

After looking at the serial core code, I don't think they are needed.
The serial core shuts down the port completely before changing the pm
state, and it also calls console_stop() which ensures that
atmel_console_write() will never get called.

And I think this is by far the best way to deal with this. As long as
the console is disabled at a higher level, we don't need those
potentially racy checks in the low-level code. So I think I'm going to
remove them before submitting this patch upstream.

If no_console_suspend is set, none of this will happen, and the clock
will keep running until it eventually gets cut off when entering slow
clock mode. This could have potential issues, but I'm not sure if it's
worth dealing with them since no_console_suspend is purely a debugging
aid.

I also suspect that disabling interrupts is unnecessary since the
serial core calls ->shutdown() which will also disable interrupts. But
it's probably safest to keep that part since the pm state may get
changed by other things than suspend.

Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/