diff mbox

[1/7] ima: on soft reboot, restore the measurement list

Message ID 1470313475-20090-2-git-send-email-zohar@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mimi Zohar Aug. 4, 2016, 12:24 p.m. UTC
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 <zohar@linux.vnet.ibm.com>
---
 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

Comments

Petko Manolov Aug. 5, 2016, 8:44 a.m. UTC | #1
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 <zohar@linux.vnet.ibm.com>
> ---
>  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.


		Petko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 5, 2016, 1:34 p.m. UTC | #2
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 <zohar@linux.vnet.ibm.com>
> > ---
> >  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.

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?

Remember this code is used once on the kexec execute and again on
reboot.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Petko Manolov Aug. 5, 2016, 3:56 p.m. UTC | #3
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 <zohar@linux.vnet.ibm.com>
> > > ---
> > >  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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Ellerman Aug. 9, 2016, 10:59 a.m. UTC | #4
Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> 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;
> +

Am I understanding it correctly that this structure is passed between kernels?

If so it's an ABI and should use types with well defined sizes, as if it was
going out to userspace, shouldn't it?

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 9, 2016, 1:01 p.m. UTC | #5
On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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;
> > +
> 
> Am I understanding it correctly that this structure is passed between kernels?

Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel.  Could the architecture (eg.
LE/BE) change between soft re-boots?

> If so it's an ABI and should use types with well defined sizes, as if it was
> going out to userspace, shouldn't it?

Ok

Mimi


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thiago Jung Bauermann Aug. 9, 2016, 1:19 p.m. UTC | #6
Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > > 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;
> > > +
> > 
> > Am I understanding it correctly that this structure is passed between
> > kernels?
> Yes, the header prefixes the measurement list, which is being passed on
> the same computer to the next kernel.  Could the architecture (eg.
> LE/BE) change between soft re-boots?

Yes. I am able to boot a BE kernel from an LE kernel with my patches. 
Whether we want to support that or not is another question...
David Laight Aug. 9, 2016, 1:35 p.m. UTC | #7
From: Thiago Jung Bauermann
> Sent: 09 August 2016 14:19
...
> > > > +/* Some details preceding the binary serialized measurement list */
> > > > +struct ima_kexec_hdr {
> > > > +	unsigned short version;
> > > > +	unsigned long buffer_size;
> > > > +	unsigned long count;
> > > > +} __packed;
> > > > +
> > >
> > > Am I understanding it correctly that this structure is passed between
> > > kernels?
> > Yes, the header prefixes the measurement list, which is being passed on
> > the same computer to the next kernel.  Could the architecture (eg.
> > LE/BE) change between soft re-boots?
> 
> Yes. I am able to boot a BE kernel from an LE kernel with my patches.

64bit kernel to/from 32bit one? (if that makes sense on the hardware).

> Whether we want to support that or not is another question...

In which case shouldn't they be annotated with the endianness??

Also why '__packed' - guarantees sub-optimal code generation.
Much better to include explicit padding to align everything.

	David

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 9, 2016, 1:55 p.m. UTC | #8
On Tue, 2016-08-09 at 10:19 -0300, Thiago Jung Bauermann wrote:
> Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> > On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> > > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > > > 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;
> > > > +
> > > 
> > > Am I understanding it correctly that this structure is passed between
> > > kernels?
> > Yes, the header prefixes the measurement list, which is being passed on
> > the same computer to the next kernel.  Could the architecture (eg.
> > LE/BE) change between soft re-boots?
> 
> Yes. I am able to boot a BE kernel from an LE kernel with my patches. 
> Whether we want to support that or not is another question...

The <securityfs/ima/binary_runtime_measurements is system architecture
dependent.  It looks like the khdr->version check in
ima_restore_measurement_list() would fail if the architecture changes.

If/when we update the binary measurement list format to support multiple
TPM PCRs, we should address the endianness as well.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 9, 2016, 2:02 p.m. UTC | #9
On Tue, 2016-08-09 at 13:35 +0000, David Laight wrote:

> Also why '__packed' - guarantees sub-optimal code generation.
> Much better to include explicit padding to align everything.

This patch set does not define a new format, but piggy backs on top of
the existing <securityfs>/ima/binary_runtime_measurements list.  The
prefixed buffer header includes a version, so that if in the future we
need to modify the format, we would be able to.

In terms of the prefixed header, how would you define the fields:
version, buffer size, number of measurements? 

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 9, 2016, 2:06 p.m. UTC | #10
On Tue, 2016-08-09 at 09:55 -0400, Mimi Zohar wrote:
> On Tue, 2016-08-09 at 10:19 -0300, Thiago Jung Bauermann wrote:
> > Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> > > On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> > > > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > > > > 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;
> > > > > +
> > > > 
> > > > Am I understanding it correctly that this structure is passed between
> > > > kernels?
> > > Yes, the header prefixes the measurement list, which is being passed on
> > > the same computer to the next kernel.  Could the architecture (eg.
> > > LE/BE) change between soft re-boots?
> > 
> > Yes. I am able to boot a BE kernel from an LE kernel with my patches. 
> > Whether we want to support that or not is another question...
> 
> The <securityfs/ima/binary_runtime_measurements is system architecture
> dependent.  It looks like the khdr->version check in
> ima_restore_measurement_list() would fail if the architecture changes.
> 
> If/when we update the binary measurement list format to support multiple
> TPM PCRs, we should address the endianness as well.

That should have been "TPM PCR banks".  TPM 2.0 allows for multiple TPM
PCR banks.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Mendoza-Jonas Aug. 9, 2016, 11:13 p.m. UTC | #11
On Tue, 2016-08-09 at 10:19 -0300, Thiago Jung Bauermann wrote:
> Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> > 
> > On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> > > 
> > > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > > > 
> > > > 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;
> > > > +
> > > 
> > > Am I understanding it correctly that this structure is passed between
> > > kernels?
> > Yes, the header prefixes the measurement list, which is being passed on
> > the same computer to the next kernel.  Could the architecture (eg.
> > LE/BE) change between soft re-boots?
> 
> Yes. I am able to boot a BE kernel from an LE kernel with my patches. 
> Whether we want to support that or not is another question...

The answer to that question is almost certainly yes - on POWER the most
immediate example would be soft-rebooting from a BE host into an LE
Petitboot kernel, or vice versa.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Ellerman Aug. 10, 2016, 3:41 a.m. UTC | #12
Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
>> On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
>> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>> > > 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;
>> > > +
>> > 
>> > Am I understanding it correctly that this structure is passed between
>> > kernels?
>> Yes, the header prefixes the measurement list, which is being passed on
>> the same computer to the next kernel.  Could the architecture (eg.
>> LE/BE) change between soft re-boots?
>
> Yes. I am able to boot a BE kernel from an LE kernel with my patches. 
> Whether we want to support that or not is another question...

Yes you must support that. BE -> LE and vice versa.

You should also consider the possibility that the next kernel is not
Linux.

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thiago Jung Bauermann Aug. 10, 2016, 5:05 a.m. UTC | #13
Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> > Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> >> On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> >> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes: 
> >> > > +/* Some details preceding the binary serialized measurement list
> >> > > */
> >> > > +struct ima_kexec_hdr {
> >> > > +	unsigned short version;
> >> > > +	unsigned long buffer_size;
> >> > > +	unsigned long count;
> >> > > +} __packed;
> >> > > +
> >> > 
> >> > Am I understanding it correctly that this structure is passed between
> >> > kernels?
> >> 
> >> Yes, the header prefixes the measurement list, which is being passed on
> >> the same computer to the next kernel.  Could the architecture (eg.
> >> LE/BE) change between soft re-boots?
> > 
> > Yes. I am able to boot a BE kernel from an LE kernel with my patches.
> > Whether we want to support that or not is another question...
> 
> Yes you must support that. BE -> LE and vice versa.

I didn't test BE - LE yet, but will do.

> You should also consider the possibility that the next kernel is not
> Linux.

If the next kernel is an ELF binary and it supports the kexec "calling 
convention", it should work too. What could possibly go wrong? I can try 
FreeBSD (I suppose it's an ELF kernel) and see what happens.
Michael Ellerman Aug. 10, 2016, 9:52 a.m. UTC | #14
Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
>> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
>> > Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
>> >> On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
>> >> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes: 
>> >> > > +/* Some details preceding the binary serialized measurement list
>> >> > > */
>> >> > > +struct ima_kexec_hdr {
>> >> > > +	unsigned short version;
>> >> > > +	unsigned long buffer_size;
>> >> > > +	unsigned long count;
>> >> > > +} __packed;
>> >> > > +
>> >> > 
>> >> > Am I understanding it correctly that this structure is passed between
>> >> > kernels?
>> >> 
>> >> Yes, the header prefixes the measurement list, which is being passed on
>> >> the same computer to the next kernel.  Could the architecture (eg.
>> >> LE/BE) change between soft re-boots?
>> > 
>> > Yes. I am able to boot a BE kernel from an LE kernel with my patches.
>> > Whether we want to support that or not is another question...
>> 
>> Yes you must support that. BE -> LE and vice versa.
>
> I didn't test BE - LE yet, but will do.

Thanks.

>> You should also consider the possibility that the next kernel is not
>> Linux.
>
> If the next kernel is an ELF binary and it supports the kexec "calling 
> convention", it should work too. What could possibly go wrong? I can try 
> FreeBSD (I suppose it's an ELF kernel) and see what happens.

At least for old style kexec (not sys_kexec_load()) I don't think it
even needs to be an ELF binary.

I think there are folks working on FreeBSD (or $?BSD), so I think the
basic kexec part works.

There's nothing (yet) that wants to use this measurement list obviously,
but it should be designed such that it could be used by an unknown
future kernel that knows the ABI.

So given what you have above, you'd use something like:

struct ima_kexec_hdr {
	u16 version;
	u16 _reserved0;
	u32 _reserved1;
	u64 buffer_size;
	u64 count;
};

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 10, 2016, 12:54 p.m. UTC | #15
On Wed, 2016-08-10 at 19:52 +1000, Michael Ellerman wrote:
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> 
> > Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
> >> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> >> > Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> >> >> On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> >> >> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes: 
> >> >> > > +/* Some details preceding the binary serialized measurement list
> >> >> > > */
> >> >> > > +struct ima_kexec_hdr {
> >> >> > > +	unsigned short version;
> >> >> > > +	unsigned long buffer_size;
> >> >> > > +	unsigned long count;
> >> >> > > +} __packed;
> >> >> > > +
> >> >> > 
> >> >> > Am I understanding it correctly that this structure is passed between
> >> >> > kernels?
> >> >> 
> >> >> Yes, the header prefixes the measurement list, which is being passed on
> >> >> the same computer to the next kernel.  Could the architecture (eg.
> >> >> LE/BE) change between soft re-boots?
> >> > 
> >> > Yes. I am able to boot a BE kernel from an LE kernel with my patches.
> >> > Whether we want to support that or not is another question...
> >> 
> >> Yes you must support that. BE -> LE and vice versa.
> >
> > I didn't test BE - LE yet, but will do.
> 
> Thanks.

Ok. There have been requests for making the binary_runtime_measurements
architecture independent.  As this was not a network facing interface,
we left it in native format.  With the kernel now consuming this data,
it makes sense for the binary_runtime_measurements to be in an
architecture independent format.

Unfortunately, as the <securityfs>/ima/binary_runtime_measurements is
not prefixed with any metadata, this change would need to be Kconfig
based, but kexec would always use the architecture independent format.

> >> You should also consider the possibility that the next kernel is not
> >> Linux.

Oh!

> > If the next kernel is an ELF binary and it supports the kexec "calling 
> > convention", it should work too. What could possibly go wrong? I can try 
> > FreeBSD (I suppose it's an ELF kernel) and see what happens.
> 
> At least for old style kexec (not sys_kexec_load()) I don't think it
> even needs to be an ELF binary.
> 
> I think there are folks working on FreeBSD (or $?BSD), so I think the
> basic kexec part works.
> 
> There's nothing (yet) that wants to use this measurement list obviously,
> but it should be designed such that it could be used by an unknown
> future kernel that knows the ABI.
> 
> So given what you have above, you'd use something like:
> 
> struct ima_kexec_hdr {
> 	u16 version;
> 	u16 _reserved0;
> 	u32 _reserved1;
> 	u64 buffer_size;
> 	u64 count;
> };
> 
> cheers

Thanks, I'll make this change.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Petko Manolov Aug. 10, 2016, 2:32 p.m. UTC | #16
On 16-08-10 08:54:36, Mimi Zohar wrote:
> On Wed, 2016-08-10 at 19:52 +1000, Michael Ellerman wrote:
> > Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> > 
> > > Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
> > >> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> > >> > Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
> > >> >> On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
> > >> >> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes: 
> > >> >> > > +/* Some details preceding the binary serialized measurement list
> > >> >> > > */
> > >> >> > > +struct ima_kexec_hdr {
> > >> >> > > +	unsigned short version;
> > >> >> > > +	unsigned long buffer_size;
> > >> >> > > +	unsigned long count;
> > >> >> > > +} __packed;
> > >> >> > > +
> > >> >> > 
> > >> >> > Am I understanding it correctly that this structure is passed between
> > >> >> > kernels?
> > >> >> 
> > >> >> Yes, the header prefixes the measurement list, which is being passed on
> > >> >> the same computer to the next kernel.  Could the architecture (eg.
> > >> >> LE/BE) change between soft re-boots?
> > >> > 
> > >> > Yes. I am able to boot a BE kernel from an LE kernel with my patches.
> > >> > Whether we want to support that or not is another question...
> > >> 
> > >> Yes you must support that. BE -> LE and vice versa.
> > >
> > > I didn't test BE - LE yet, but will do.
> > 
> > Thanks.
> 
> Ok. There have been requests for making the binary_runtime_measurements
> architecture independent.  As this was not a network facing interface,
> we left it in native format.  With the kernel now consuming this data,
> it makes sense for the binary_runtime_measurements to be in an
> architecture independent format.
> 
> Unfortunately, as the <securityfs>/ima/binary_runtime_measurements is
> not prefixed with any metadata, this change would need to be Kconfig
> based, but kexec would always use the architecture independent format.
> 
> > >> You should also consider the possibility that the next kernel is not
> > >> Linux.
> 
> Oh!
> 
> > > If the next kernel is an ELF binary and it supports the kexec "calling 
> > > convention", it should work too. What could possibly go wrong? I can try 
> > > FreeBSD (I suppose it's an ELF kernel) and see what happens.
> > 
> > At least for old style kexec (not sys_kexec_load()) I don't think it
> > even needs to be an ELF binary.
> > 
> > I think there are folks working on FreeBSD (or $?BSD), so I think the
> > basic kexec part works.
> > 
> > There's nothing (yet) that wants to use this measurement list obviously,
> > but it should be designed such that it could be used by an unknown
> > future kernel that knows the ABI.
> > 
> > So given what you have above, you'd use something like:
> > 
> > struct ima_kexec_hdr {
> > 	u16 version;
> > 	u16 _reserved0;
> > 	u32 _reserved1;
> > 	u64 buffer_size;
> > 	u64 count;
> > };
> > 
> > cheers
> 
> Thanks, I'll make this change.

I would suggest:

struct ima_kexec_hdr {
	u64 buffer_size;
	u64 count;
	u16 version;
};

and let the compiler add the proper padding, depending on the architecture.  On 
32bit machine we'll have 4 bytes smaller allocations (compared to 64bit) while 
retaining the same functionality.


cheers,
Petko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight Aug. 10, 2016, 2:40 p.m. UTC | #17
From: Linuxppc-dev [mailto:linuxppc-dev-bounces+david.laight=aculab.com@lists.ozlabs.org] On Behalf Of
> > > So given what you have above, you'd use something like:
> > >
> > > struct ima_kexec_hdr {
> > > 	u16 version;
> > > 	u16 _reserved0;
> > > 	u32 _reserved1;
> > > 	u64 buffer_size;
> > > 	u64 count;
> > > };
> > >
> > > cheers
> >
> > Thanks, I'll make this change.
> 
> I would suggest:
> 
> struct ima_kexec_hdr {
> 	u64 buffer_size;
> 	u64 count;
> 	u16 version;
> };
> 
> and let the compiler add the proper padding, depending on the architecture.  On
> 32bit machine we'll have 4 bytes smaller allocations (compared to 64bit) while
> retaining the same functionality.

AAAArrrrgggg.....

That doesn't work for 32bit applications on 64bit hosts.
The extra bytes will make 0 difference to the allocation cost and
lots to the processing.

	David

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Petko Manolov Aug. 10, 2016, 3:48 p.m. UTC | #18
On 16-08-10 14:40:13, David Laight wrote:
> From: Linuxppc-dev [mailto:linuxppc-dev-bounces+david.laight=aculab.com@lists.ozlabs.org] On Behalf Of
> > > > So given what you have above, you'd use something like:
> > > >
> > > > struct ima_kexec_hdr {
> > > > 	u16 version;
> > > > 	u16 _reserved0;
> > > > 	u32 _reserved1;
> > > > 	u64 buffer_size;
> > > > 	u64 count;
> > > > };
> > > >
> > > > cheers
> > >
> > > Thanks, I'll make this change.
> > 
> > I would suggest:
> > 
> > struct ima_kexec_hdr {
> > 	u64 buffer_size;
> > 	u64 count;
> > 	u16 version;
> > };
> > 
> > and let the compiler add the proper padding, depending on the architecture.  On
> > 32bit machine we'll have 4 bytes smaller allocations (compared to 64bit) while
> > retaining the same functionality.
> 
> AAAArrrrgggg.....
> 
> That doesn't work for 32bit applications on 64bit hosts.

Which part won't work?

> The extra bytes will make 0 difference to the allocation cost and lots to the 
> processing.

An example?


		Petko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
@@ -122,6 +129,9 @@  int ima_init_crypto(void);
 void ima_putc(struct seq_file *m, void *data, int datalen);
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
 struct ima_template_desc *ima_template_desc_current(void);
+void ima_load_kexec_buffer(void);
+int ima_restore_measurement_entry(struct ima_template_entry *entry);
+int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_init_template(void);
 
 /*
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 32912bd..3ba0ca4 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -128,6 +128,8 @@  int __init ima_init(void)
 	if (rc != 0)
 		return rc;
 
+	ima_load_kexec_buffer();
+
 	rc = ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	if (rc != 0)
 		return rc;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
new file mode 100644
index 0000000..6a046ad
--- /dev/null
+++ b/security/integrity/ima/ima_kexec.c
@@ -0,0 +1,55 @@ 
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Authors:
+ * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
+ * Mimi Zohar <zohar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/fcntl.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/seq_file.h>
+#include <linux/rculist.h>
+#include <linux/rcupdate.h>
+#include <linux/parser.h>
+#include <linux/vmalloc.h>
+#include <linux/kexec.h>
+#include <linux/reboot.h>
+
+#include "ima.h"
+
+/*
+ * Restore the measurement list from the previous kernel.
+ */
+void ima_load_kexec_buffer(void)
+{
+	void *kexec_buffer = NULL;
+	size_t kexec_buffer_size = 0;
+	int rc;
+
+	rc = kexec_get_handover_buffer(&kexec_buffer, &kexec_buffer_size);
+	switch (rc) {
+	case 0:
+		rc = ima_restore_measurement_list(kexec_buffer_size,
+						  kexec_buffer);
+		if (rc != 0)
+			pr_err("Failed to restore the measurement list: %d\n",
+				rc);
+
+		kexec_free_handover_buffer();
+		break;
+	case -ENOTSUPP:
+		pr_debug("Restoring the measurement list not supported\n");
+		break;
+	case -ENOENT:
+		pr_debug("No measurement list to restore\n");
+		break;
+	default:
+		pr_debug("Error restoring the measurement list: %d\n", rc);
+	}
+}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 32f6ac0..4b1bb77 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -149,3 +149,13 @@  out:
 			    op, audit_cause, result, audit_info);
 	return result;
 }
+
+int ima_restore_measurement_entry(struct ima_template_entry *entry)
+{
+	int result = 0;
+
+	mutex_lock(&ima_extend_list_mutex);
+	result = ima_add_digest_entry(entry);
+	mutex_unlock(&ima_extend_list_mutex);
+	return result;
+}
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index febd12e..c6510f0 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -205,3 +205,174 @@  int __init ima_init_template(void)
 
 	return result;
 }
+
+static int ima_restore_template_data(struct ima_template_desc *template_desc,
+				     void *template_data,
+				     int template_data_size,
+				     struct ima_template_entry **entry)
+{
+	struct binary_field_data {
+		u32 len;
+		u8 data[0];
+	} __packed;
+
+	struct binary_field_data *field_data;
+	int offset = 0;
+	int ret = 0;
+	int i;
+
+	*entry = kzalloc(sizeof(**entry) +
+		    template_desc->num_fields * sizeof(struct ima_field_data),
+		    GFP_NOFS);
+	if (!*entry)
+		return -ENOMEM;
+
+	(*entry)->template_desc = template_desc;
+	for (i = 0; i < template_desc->num_fields; i++) {
+		field_data = template_data + offset;
+
+		/* Each field of the template data is prefixed with a length. */
+		if (offset > (template_data_size - sizeof(field_data->len))) {
+			pr_err("Restoring the template field failed\n");
+			ret = -EINVAL;
+			break;
+		}
+		offset += sizeof(field_data->len);
+
+		if (offset > (template_data_size - field_data->len)) {
+			pr_err("Restoring the template field data failed\n");
+			ret = -EINVAL;
+			break;
+		}
+		offset += field_data->len;
+
+		(*entry)->template_data[i].len = field_data->len;
+		(*entry)->template_data_len += sizeof(field_data->len);
+
+		(*entry)->template_data[i].data =
+			kzalloc(field_data->len + 1, GFP_KERNEL);
+		if (!(*entry)->template_data[i].data) {
+			ret = -ENOMEM;
+			break;
+		}
+		memcpy((*entry)->template_data[i].data, field_data->data,
+			field_data->len);
+		(*entry)->template_data_len += field_data->len;
+	}
+
+	if (ret < 0) {
+		ima_free_template_entry(*entry);
+		*entry = NULL;
+	}
+
+	return ret;
+}
+
+#define MAX_TEMPLATE_NAME_LEN 30
+
+/* Restore the serialized binary measurement list without extending PCRs. */
+int ima_restore_measurement_list(loff_t size, void *buf)
+{
+	struct binary_hdr_v1 {
+		u32 pcr;
+		u8 digest[TPM_DIGEST_SIZE];
+		u32 template_name_len;
+		char template_name[0];
+	} __packed;
+	char template_name[MAX_TEMPLATE_NAME_LEN];
+
+	struct binary_data_v1 {
+		u32 template_data_size;
+		char template_data[0];
+	} __packed;
+
+	struct ima_kexec_hdr *khdr = buf;
+	struct binary_hdr_v1 *hdr_v1;
+	struct binary_data_v1 *data_v1;
+
+	void *bufp = buf + sizeof(*khdr);
+	void *bufendp = buf + khdr->buffer_size;
+	struct ima_template_entry *entry;
+	struct ima_template_desc *template_desc;
+	unsigned long count = 0;
+	int ret = 0;
+
+	if (!buf || size < sizeof(*khdr))
+		return 0;
+
+	if (khdr->version != 1) {
+		pr_err("attempting to restore a incompatible measurement list");
+		return 0;
+	}
+
+	/*
+	 * ima kexec buffer prefix: version, buffer size, count
+	 * v1 format: pcr, digest, template-name-len, template-name,
+	 *	      template-data-size, template-data
+	 */
+	while ((bufp < bufendp) && (count++ < khdr->count)) {
+		if (count > ULONG_MAX - 1) {
+			pr_err("attempting to restore too many measurements");
+			ret = -EINVAL;
+		}
+
+		hdr_v1 = bufp;
+		if ((hdr_v1->template_name_len > MAX_TEMPLATE_NAME_LEN) ||
+		    ((bufp + hdr_v1->template_name_len) > bufendp)) {
+			pr_err("attempting to restore a template name \
+				that is too long\n");
+			ret = -EINVAL;
+			break;
+		}
+		bufp += sizeof(*hdr_v1);
+
+		/* template name is not null terminated */
+		memcpy(template_name, bufp, hdr_v1->template_name_len);
+		template_name[hdr_v1->template_name_len] = 0;
+
+		if (strcmp(template_name, "ima") == 0) {
+			pr_err("attempting to restore an unsupported \
+				template \"%s\" failed\n", template_name);
+			ret = -EINVAL;
+			break;
+		}
+		data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len;
+
+		/* get template format */
+		template_desc = lookup_template_desc(template_name);
+		if (!template_desc) {
+			pr_err("template \"%s\" not found\n", template_name);
+			ret = -EINVAL;
+			break;
+		}
+
+		if (bufp > (bufendp - sizeof(data_v1->template_data_size))) {
+			pr_err("restoring the template data size failed\n");
+			ret = -EINVAL;
+			break;
+		}
+		bufp += (u_int8_t) sizeof(data_v1->template_data_size);
+
+		if (bufp > (bufendp - data_v1->template_data_size)) {
+			pr_err("restoring the template data failed\n");
+			ret = -EINVAL;
+			break;
+		}
+
+		ret = ima_restore_template_data(template_desc,
+						data_v1->template_data,
+						data_v1->template_data_size,
+						&entry);
+		if (ret < 0)
+			break;
+
+		memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE);
+		entry->pcr = hdr_v1->pcr;
+		ret = ima_restore_measurement_entry(entry);
+		if (ret < 0)
+			break;
+
+		bufp += data_v1->template_data_size;
+	}
+	return ret;
+}