linux-kernel - Re: [RFT 1/2] printk: make dynamic kernel ring buffer alignemnt explicit

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53A1B693.2000609@wwwdotorg.org>
Date:	Wed, 18 Jun 2014 09:56:03 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>, hpa@...ux.intel.com,
	akpm@...ux-foundation.org
CC:	linux-kernel@...r.kernel.org,
	"Luis R. Rodriguez" <mcgrof@...e.com>,
	Andrew Lunn <andrew@...n.ch>, Michal Hocko <mhocko@...e.cz>,
	Petr Mladek <pmladek@...e.cz>, Joe Perches <joe@...ches.com>,
	Arun KS <arunks.linux@...il.com>,
	Kees Cook <keescook@...omium.org>,
	Davidlohr Bueso <davidlohr@...com>,
	Chris Metcalf <cmetcalf@...era.com>
Subject: Re: [RFT 1/2] printk: make dynamic kernel ring buffer alignemnt explicit

On 06/18/2014 05:14 AM, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@...e.com>
> 
> We have to consider alignment for the ring buffer both for the
> default static size, and then also for when an dynamic allocation
> is made when the log_buf_len=n kernel parameter is passed to set
> the size specifically to a size larger than the default size set
> by the architecture through CONFIG_LOG_BUF_SHIFT.
> 
> The default static kernel ring buffer can be aligned properly if
> architectures set CONFIG_LOG_BUF_SHIFT properly, we provide ranges
> for the size though so even if CONFIG_LOG_BUF_SHIFT has a sensible
> aligned value it can be reduced to a non aligned value. Commit
> 6ebb017de9 by Andrew ensures the static buffer is always aligned
> and the decision of alignment is done by the compiler by using
> __alignof__(struct log) (curious what value caused the crash?).

IIRC the issue was that __log_buf's type is char[] so without the
__aligned it could have any alignment at all, e.g. 1 or 2. However,
struct printk_log is stored in the buffer rather than just char*, and so
if __log_buf isn't aligned to the required alignment for that structure,
that can caused unaligned accesses to fields in the structure, which
isn't supported on ARM in at least some cases.

As such, I think the change to setup_log_buf() in this patch makes sense
(although I suppose in practice memblock_virt_alloc() probably has some
minimum internal alignment that dwards LOG_ALIGN, but that's an
implementation detail we shouldn't rely on).
--
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/