From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1422784AbcHEP4b (ORCPT ); Fri, 5 Aug 2016 11:56:31 -0400 Received: from lan.nucleusys.com ([92.247.61.126]:37296 "EHLO zztop.nucleusys.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1422757AbcHEP43 (ORCPT ); Fri, 5 Aug 2016 11:56:29 -0400 Date: Fri, 5 Aug 2016 18:56:02 +0300 From: Petko Manolov To: Mimi Zohar Cc: linux-security-module@vger.kernel.org, linux-ima-devel@lists.sourceforge.net, Dave Young , kexec@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Thiago Jung Bauermann Subject: Re: [PATCH 1/7] ima: on soft reboot, restore the measurement list Message-ID: <20160805155602.GA1738@localhost> Mail-Followup-To: Mimi Zohar , linux-security-module@vger.kernel.org, linux-ima-devel@lists.sourceforge.net, Dave Young , kexec@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Thiago Jung Bauermann References: <1470313475-20090-1-git-send-email-zohar@linux.vnet.ibm.com> <1470313475-20090-2-git-send-email-zohar@linux.vnet.ibm.com> <20160805084425.GA7572@localhost> <1470404078.2471.123.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1470404078.2471.123.camel@linux.vnet.ibm.com> User-Agent: Mutt/1.6.0 (2016-04-01) X-Spam-Score: -1.0 (-) X-Spam-Report: Spam detection software, running on the system "zztop.nucleusys.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: On 16-08-05 09:34:38, Mimi Zohar wrote: > Hi Petko, > > Thank you for review! > > On Fri, 2016-08-05 at 11:44 +0300, Petko Manolov wrote: > > On 16-08-04 08:24:29, Mimi Zohar wrote: > > > The TPM PCRs are only reset on a hard reboot. In order to validate a > > > TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list > > > of the running kernel must be saved and restored on boot. This patch > > > restores the measurement list. > > > > > > Changelog: > > > - call ima_load_kexec_buffer() (Thiago) > > > > > > Signed-off-by: Mimi Zohar > > > --- > > > security/integrity/ima/Makefile | 1 + > > > security/integrity/ima/ima.h | 10 ++ > > > security/integrity/ima/ima_init.c | 2 + > > > security/integrity/ima/ima_kexec.c | 55 +++++++++++ > > > security/integrity/ima/ima_queue.c | 10 ++ > > > security/integrity/ima/ima_template.c | 171 ++++++++++++++++++++++++++++++++++ > > > 6 files changed, 249 insertions(+) > > > create mode 100644 security/integrity/ima/ima_kexec.c > > > > > > diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile > > > index c34599f..c0ce7b1 100644 > > > --- a/security/integrity/ima/Makefile > > > +++ b/security/integrity/ima/Makefile > > > @@ -8,4 +8,5 @@ obj-$(CONFIG_IMA) += ima.o > > > ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \ > > > ima_policy.o ima_template.o ima_template_lib.o ima_buffer.o > > > ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o > > > +ima-$(CONFIG_KEXEC_FILE) += ima_kexec.o > > > obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o > > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > > > index b5728da..84e8d36 100644 > > > --- a/security/integrity/ima/ima.h > > > +++ b/security/integrity/ima/ima.h > > > @@ -102,6 +102,13 @@ struct ima_queue_entry { > > > }; > > > extern struct list_head ima_measurements; /* list of all measurements */ > > > > > > +/* Some details preceding the binary serialized measurement list */ > > > +struct ima_kexec_hdr { > > > + unsigned [...] Content analysis details: (-1.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP 0.0 TVD_RCVD_IP Message was received from an IP address Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16-08-05 09:34:38, Mimi Zohar wrote: > Hi Petko, > > Thank you for review! > > On Fri, 2016-08-05 at 11:44 +0300, Petko Manolov wrote: > > On 16-08-04 08:24:29, Mimi Zohar wrote: > > > The TPM PCRs are only reset on a hard reboot. In order to validate a > > > TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list > > > of the running kernel must be saved and restored on boot. This patch > > > restores the measurement list. > > > > > > Changelog: > > > - call ima_load_kexec_buffer() (Thiago) > > > > > > Signed-off-by: Mimi Zohar > > > --- > > > security/integrity/ima/Makefile | 1 + > > > security/integrity/ima/ima.h | 10 ++ > > > security/integrity/ima/ima_init.c | 2 + > > > security/integrity/ima/ima_kexec.c | 55 +++++++++++ > > > security/integrity/ima/ima_queue.c | 10 ++ > > > security/integrity/ima/ima_template.c | 171 ++++++++++++++++++++++++++++++++++ > > > 6 files changed, 249 insertions(+) > > > create mode 100644 security/integrity/ima/ima_kexec.c > > > > > > diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile > > > index c34599f..c0ce7b1 100644 > > > --- a/security/integrity/ima/Makefile > > > +++ b/security/integrity/ima/Makefile > > > @@ -8,4 +8,5 @@ obj-$(CONFIG_IMA) += ima.o > > > ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \ > > > ima_policy.o ima_template.o ima_template_lib.o ima_buffer.o > > > ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o > > > +ima-$(CONFIG_KEXEC_FILE) += ima_kexec.o > > > obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o > > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > > > index b5728da..84e8d36 100644 > > > --- a/security/integrity/ima/ima.h > > > +++ b/security/integrity/ima/ima.h > > > @@ -102,6 +102,13 @@ struct ima_queue_entry { > > > }; > > > extern struct list_head ima_measurements; /* list of all measurements */ > > > > > > +/* Some details preceding the binary serialized measurement list */ > > > +struct ima_kexec_hdr { > > > + unsigned short version; > > > + unsigned long buffer_size; > > > + unsigned long count; > > > +} __packed; > > > > Unless there is no real need for this structure to be packed i suggest > > dropping the attribute. When referenced through pointer 32bit ARM and MIPS > > (and likely all other 32bit RISC CPUs) use rather inefficient byte loads and > > stores. > > > > Worse, if, for example, ->count is going to be read/written concurrently > > from multiple threads we get torn loads/stores thus losing atomicity of the > > access. > > This header is used to prefix the serialized binary measurement list with some > meta-data about the measurement list being restored. Unfortunately > kexec_get_handover_buffer() returns the segment size, not the actual ima > measurement list buffer size. The header info is set using memcpy() once in > ima_dump_measurement_list() and then the fields are used in > ima_restore_measurement_list() to verify the buffer. As long as there is no concurrent reads/writes this should be OK. > The binary runtime measurement list is packed, so the other two structures - > binary_hdr_v1 and binary_data_v1 - must be packed. Does it make sense for > this header not to be packed as well? Would copying the header fields to > local variables before being used solve your concern? Copying to aligned variables would be necessary only if: a) some sort of atomicity is needed, and/or б) speed is of concern; > Remember this code is used once on the kexec execute and again on reboot. If we don't need a) _and_ b) then you don't need to bother. Petko