From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751023AbdE1VTZ (ORCPT ); Sun, 28 May 2017 17:19:25 -0400 Received: from mail-io0-f172.google.com ([209.85.223.172]:35675 "EHLO mail-io0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750858AbdE1VTY (ORCPT ); Sun, 28 May 2017 17:19:24 -0400 MIME-Version: 1.0 In-Reply-To: <1496003387-3184-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> References: <1495883858-3336-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <1496003387-3184-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> From: Kees Cook Date: Sun, 28 May 2017 14:19:22 -0700 X-Google-Sender-Auth: qDgW5Fya8CBIAcEKnmlU13r9Oa4 Message-ID: Subject: Re: [PATCH v2] LSM: Convert security_hook_heads into explicit array of struct list_head To: Tetsuo Handa Cc: linux-security-module , LKML , "kernel-hardening@lists.openwall.com" , Casey Schaufler , Christoph Hellwig , Igor Stoppa , James Morris , Paul Moore , Stephen Smalley Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, May 28, 2017 at 1:29 PM, Tetsuo Handa wrote: > Commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon > registration.") treats "struct security_hook_heads" as an implicit array > of "struct list_head" so that we can eliminate code for static > initialization. Although we haven't encountered compilers which do not > treat sizeof(security_hook_heads) != sizeof(struct list_head) * > (sizeof(security_hook_heads) / sizeof(struct list_head)), Casey does not > like the assumption that a structure of N elements can be assumed to be > the same as an array of N elements. > > Now that Kees found that randstruct complains about such casting > > security/security.c: In function 'security_init': > security/security.c:59:20: note: found mismatched op0 struct pointer types: 'struct list_head' and 'struct security_hook_heads' > > struct list_head *list = (struct list_head *) &security_hook_heads; > > and Christoph thinks that we should fix it rather than make randstruct > whitelist it, this patch fixes it. > > It would be possible to revert commit 3dfc9b02864b19f4, but this patch > converts security_hook_heads into an explicit array of struct list_head > by introducing an enum, due to reasons explained below. > > Igor proposed a sealable memory allocator, and the LSM hooks > ("struct security_hook_heads security_hook_heads" and > "struct security_hook_list ...[]") will benefit from that allocator via > protection using set_memory_ro()/set_memory_rw(), and that allocator > will remove CONFIG_SECURITY_WRITABLE_HOOKS config option. Thus, we will > likely be moving to that direction. > > This means that these structures will be allocated at run time using > that allocator, and therefore the address of these structures will be > determined at run time rather than compile time. > > But currently, LSM_HOOK_INIT() macro depends on the address of > security_hook_heads being known at compile time. If we use an enum > so that LSM_HOOK_INIT() macro does not need to know absolute address of > security_hook_heads, it will help us to use that allocator for LSM hooks. > > As a result of introducing an enum, security_hook_heads becomes a local > variable. In order to pass 80 columns check by scripts/checkpatch.pl , > rename security_hook_heads to hook_heads. > > Signed-off-by: Tetsuo Handa > Cc: Kees Cook > Cc: Paul Moore > Cc: Stephen Smalley > Cc: Casey Schaufler > Cc: James Morris > Cc: Igor Stoppa > Cc: Christoph Hellwig Looks good to me; thanks for persisting! :) Acked-by: Kees Cook -- Kees Cook Pixel Security