Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

2017-10-04 Thread Jarkko Sakkinen 
On Wed, Oct 04, 2017 at 01:51:13PM +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 29, 2017 at 08:16:17PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
> > > On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
> > >  wrote:
> > > > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
> > > >> With TPM 2.0 specification, the event logs may only be accessible by
> > > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area 
> > > >> to
> > > >> a new Linux-specific EFI configuration table so it remains accessible
> > > >> once booted.
> > > >>
> > > >> When calling this service, it is possible to specify the expected 
> > > >> format
> > > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
> > > >> the
> > > >> first format is retrieved.
> > > >>
> > > >> Signed-off-by: Thiebaud Weksteen 
> > > >
> > > > Does not apply:
> > > >
> > > > Applying: tpm: move tpm_eventlog.h outside of drivers folder
> > > > Applying: tpm: rename event log provider files
> > > > Applying: tpm: add event log format version
> > > > Applying: efi: call get_event_log before ExitBootServices
> > > > error: sha1 information is lacking or useless 
> > > > (drivers/firmware/efi/efi.c).
> > > > error: could not build fake ancestor
> > > > Patch failed at 0004 efi: call get_event_log before ExitBootServices
> > > > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > > > When you have resolved this problem, run "git am --continue".
> > > > If you prefer to skip this patch, run "git am --skip" instead.
> > > > To restore the original branch and stop patching, run "git am --abort".
> > > >
> > > > Just rebased my tree to the latest security-next.
> > > 
> > > It applies fine on security/next-general which is more up-to-date.
> > > (security/next does not include
> > > ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
> > > based)
> > 
> > Thanks, my bad, I though that I had it updated.
> > 
> > I'll update my tree and retry.
> > 
> > /Jarkko
> 
> My master is up to date with security/next.
> 
> Still get the same result:
> 
> $ git am -3 
> ~/Downloads/v3-4-5-efi-call-get_event_log-before-ExitBootServices.patch
> Applying: efi: call get_event_log before ExitBootServices
> error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
> error: could not build fake ancestor
> Patch failed at 0001 efi: call get_event_log before ExitBootServices
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Maybe you have some other trees fetched in your local GIT so that it
> finds the ancestors? Anyway, cannot test this at this point.
> 
> /Jarkko

I pushed the first three patches to my master as they looked OK. You
should still consider them unreviewed.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

2017-10-04 Thread Thiebaud Weksteen 
On Wed, Oct 4, 2017 at 12:51 PM, Jarkko Sakkinen
 wrote:
> On Fri, Sep 29, 2017 at 08:16:17PM +0300, Jarkko Sakkinen wrote:
>> On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
>> > On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
>> >  wrote:
>> > > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
>> > >> With TPM 2.0 specification, the event logs may only be accessible by
>> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
>> > >> a new Linux-specific EFI configuration table so it remains accessible
>> > >> once booted.
>> > >>
>> > >> When calling this service, it is possible to specify the expected format
>> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
>> > >> the
>> > >> first format is retrieved.
>> > >>
>> > >> Signed-off-by: Thiebaud Weksteen 
>> > >
>> > > Does not apply:
>> > >
>> > > Applying: tpm: move tpm_eventlog.h outside of drivers folder
>> > > Applying: tpm: rename event log provider files
>> > > Applying: tpm: add event log format version
>> > > Applying: efi: call get_event_log before ExitBootServices
>> > > error: sha1 information is lacking or useless 
>> > > (drivers/firmware/efi/efi.c).
>> > > error: could not build fake ancestor
>> > > Patch failed at 0004 efi: call get_event_log before ExitBootServices
>> > > The copy of the patch that failed is found in: .git/rebase-apply/patch
>> > > When you have resolved this problem, run "git am --continue".
>> > > If you prefer to skip this patch, run "git am --skip" instead.
>> > > To restore the original branch and stop patching, run "git am --abort".
>> > >
>> > > Just rebased my tree to the latest security-next.
>> >
>> > It applies fine on security/next-general which is more up-to-date.
>> > (security/next does not include
>> > ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
>> > based)
>>
>> Thanks, my bad, I though that I had it updated.
>>
>> I'll update my tree and retry.
>>
>> /Jarkko
>
> My master is up to date with security/next.
>
> Still get the same result:
>
> $ git am -3 
> ~/Downloads/v3-4-5-efi-call-get_event_log-before-ExitBootServices.patch
> Applying: efi: call get_event_log before ExitBootServices
> error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
> error: could not build fake ancestor
> Patch failed at 0001 efi: call get_event_log before ExitBootServices
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Maybe you have some other trees fetched in your local GIT so that it
> finds the ancestors? Anyway, cannot test this at this point.
>
> /Jarkko

The security/next branch still does not contain the commit I mentioned
(ccc829ba3624beb9a703fc995d016b836d9eead8), which is already part of
torvalds/master now.

 $ git branch -a --contains ccc829ba3624beb9a703fc995d016b836d9eead8
  efi_tpm2_eventlog
  master
  remotes/linux-next/akpm
  remotes/linux-next/akpm-base
  remotes/linux-next/master
  remotes/linux-next/stable
  remotes/security/fixes-v4.14-rc3
  remotes/security/fixes-v4.14-rc4
  remotes/security/next-general
  remotes/security/next-testing
  remotes/torvalds/master

Is there any reason why you are trying to merge on that specific
branch and not next-general or next-testing? Would you know the
purpose of all these next-* branches?

Thanks,
Thiebaud
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

2017-10-04 Thread Jarkko Sakkinen 
On Fri, Sep 29, 2017 at 08:16:17PM +0300, Jarkko Sakkinen wrote:
> On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
> > On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
> >  wrote:
> > > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
> > >> With TPM 2.0 specification, the event logs may only be accessible by
> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> > >> a new Linux-specific EFI configuration table so it remains accessible
> > >> once booted.
> > >>
> > >> When calling this service, it is possible to specify the expected format
> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
> > >> the
> > >> first format is retrieved.
> > >>
> > >> Signed-off-by: Thiebaud Weksteen 
> > >
> > > Does not apply:
> > >
> > > Applying: tpm: move tpm_eventlog.h outside of drivers folder
> > > Applying: tpm: rename event log provider files
> > > Applying: tpm: add event log format version
> > > Applying: efi: call get_event_log before ExitBootServices
> > > error: sha1 information is lacking or useless 
> > > (drivers/firmware/efi/efi.c).
> > > error: could not build fake ancestor
> > > Patch failed at 0004 efi: call get_event_log before ExitBootServices
> > > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > > When you have resolved this problem, run "git am --continue".
> > > If you prefer to skip this patch, run "git am --skip" instead.
> > > To restore the original branch and stop patching, run "git am --abort".
> > >
> > > Just rebased my tree to the latest security-next.
> > 
> > It applies fine on security/next-general which is more up-to-date.
> > (security/next does not include
> > ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
> > based)
> 
> Thanks, my bad, I though that I had it updated.
> 
> I'll update my tree and retry.
> 
> /Jarkko

My master is up to date with security/next.

Still get the same result:

$ git am -3 
~/Downloads/v3-4-5-efi-call-get_event_log-before-ExitBootServices.patch
Applying: efi: call get_event_log before ExitBootServices
error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
error: could not build fake ancestor
Patch failed at 0001 efi: call get_event_log before ExitBootServices
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Maybe you have some other trees fetched in your local GIT so that it
finds the ancestors? Anyway, cannot test this at this point.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 19/27] x86: assembly, make some functions local

2017-10-04 Thread Ard Biesheuvel 
Hello Jiri,

On 4 October 2017 at 08:22, Jiri Slaby  wrote:
> On 10/02/2017, 02:48 PM, Ard Biesheuvel wrote:
>> On 2 October 2017 at 10:12, Jiri Slaby  wrote:
>>> There is a couple of assembly functions, which are invoked only locally
>>> in the file they are defined. In C, we mark them "static". In assembly,
>>> annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
>>> ENDPROC to SYM_{FUNC,CODE}_END too). Whether FUNC or CODE depends on
>>> ENDPROC/END for a particular function (C or non-C).
>>>
>>
>> I wasn't cc'ed on the cover letter, so I am missing the rationale of
>> replacing ENTRY/ENDPROC with other macros.
>
> There was no cover letter. I am attaching what is in PATCH 1/27 instead:
> Introduce new C macros for annotations of functions and data in
> assembly. There is a long-standing mess in macros like ENTRY, END,
> ENDPROC and similar. They are used in different manners and sometimes
> incorrectly.
>

I must say, I don't share this sentiment.

In arm64, we use ENTRY/ENDPROC for functions with external linkage,
and the bare symbol name/ENDPROC for functions with local linkage. I
guess we could add ENDOBJECT if we wanted to, but we never really felt
the need.

> So introduce macros with clear use to annotate assembly as follows:
>
> a) Support macros for the ones below
>SYM_T_FUNC -- type used by assembler to mark functions
>SYM_T_OBJECT -- type used by assembler to mark data
>SYM_T_NONE -- type used by assembler to mark entries of unknown type
>

Is is necessary to mark an entry as having no type? What is the
default type for an unmarked entry?

>They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE
>respectively. According to the gas manual, this is the most portable
>way. I am not sure about other assemblers, so we can switch this back
>to %function and %object if this turns into a problem. Architectures
>can also override them by something like ", @function" if they need.
>
>SYM_A_ALIGN, SYM_A_NONE -- align the symbol?
>SYM_V_GLOBAL, SYM_V_WEAK, SYM_V_LOCAL -- visibility of symbols
>

Linkage != visibility

> b) Mostly internal annotations, used by the ones below
>SYM_ENTRY -- use only if you have to (for non-paired symbols)
>SYM_START -- use only if you have to (for paired symbols)
>SYM_END -- use only if you have to (for paired symbols)
>
> c) Annotations for code
>SYM_FUNC_START_LOCAL_ALIAS -- use where there are two local names for
> one function
>SYM_FUNC_START_ALIAS -- use where there are two global names for one
> function
>SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function
>
>SYM_FUNC_START -- use for global functions
>SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment
>SYM_FUNC_START_LOCAL -- use for local functions
>SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o
> alignment
>SYM_FUNC_START_WEAK -- use for weak functions
>SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment
>SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START,
> SYM_FUNC_START_WEAK, ...
>
>SYM_FUNC_INNER_LABEL -- only for labels in the middle of functions
>SYM_FUNC_INNER_LABEL_NOALIGN -- only for labels in the middle of
> functions, w/o alignment
>
>For functions with special (non-C) calling conventions:
>SYM_CODE_START -- use for non-C (special) functions
>SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o
> alignment
>SYM_CODE_START_LOCAL -- use for local non-C (special) functions
>SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special)
> functions, w/o alignment
>SYM_CODE_END -- the end of SYM_CODE_START_LOCAL or SYM_CODE_START
>
>SYM_CODE_INNER_LABEL -- only for labels in the middle of code
>SYM_CODE_INNER_LABEL_NOALIGN -- only for labels in the middle of code
>
> d) For data
>SYM_DATA_START -- global data symbol
>SYM_DATA_END -- the end of the SYM_DATA_START symbol
>SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol
>SYM_DATA_SIMPLE -- start+end wrapper around simple global data
>SYM_DATA_SIMPLE_LOCAL -- start+end wrapper around simple local data
>

I am sorry but I think this is terrible. Do we really need 20+ new
macros to wrap every single assembler directive involved in defining
symbols and setting their attributes?

Is this issue you are solving widely perceived as a problem?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 19/27] x86: assembly, make some functions local

2017-10-04 Thread Jiri Slaby 
On 10/02/2017, 02:48 PM, Ard Biesheuvel wrote:
> On 2 October 2017 at 10:12, Jiri Slaby  wrote:
>> There is a couple of assembly functions, which are invoked only locally
>> in the file they are defined. In C, we mark them "static". In assembly,
>> annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
>> ENDPROC to SYM_{FUNC,CODE}_END too). Whether FUNC or CODE depends on
>> ENDPROC/END for a particular function (C or non-C).
>>
> 
> I wasn't cc'ed on the cover letter, so I am missing the rationale of
> replacing ENTRY/ENDPROC with other macros.

There was no cover letter. I am attaching what is in PATCH 1/27 instead:
Introduce new C macros for annotations of functions and data in
assembly. There is a long-standing mess in macros like ENTRY, END,
ENDPROC and similar. They are used in different manners and sometimes
incorrectly.

So introduce macros with clear use to annotate assembly as follows:

a) Support macros for the ones below
   SYM_T_FUNC -- type used by assembler to mark functions
   SYM_T_OBJECT -- type used by assembler to mark data
   SYM_T_NONE -- type used by assembler to mark entries of unknown type

   They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE
   respectively. According to the gas manual, this is the most portable
   way. I am not sure about other assemblers, so we can switch this back
   to %function and %object if this turns into a problem. Architectures
   can also override them by something like ", @function" if they need.

   SYM_A_ALIGN, SYM_A_NONE -- align the symbol?
   SYM_V_GLOBAL, SYM_V_WEAK, SYM_V_LOCAL -- visibility of symbols

b) Mostly internal annotations, used by the ones below
   SYM_ENTRY -- use only if you have to (for non-paired symbols)
   SYM_START -- use only if you have to (for paired symbols)
   SYM_END -- use only if you have to (for paired symbols)

c) Annotations for code
   SYM_FUNC_START_LOCAL_ALIAS -- use where there are two local names for
one function
   SYM_FUNC_START_ALIAS -- use where there are two global names for one
function
   SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function

   SYM_FUNC_START -- use for global functions
   SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment
   SYM_FUNC_START_LOCAL -- use for local functions
   SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o
alignment
   SYM_FUNC_START_WEAK -- use for weak functions
   SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment
   SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START,
SYM_FUNC_START_WEAK, ...

   SYM_FUNC_INNER_LABEL -- only for labels in the middle of functions
   SYM_FUNC_INNER_LABEL_NOALIGN -- only for labels in the middle of
functions, w/o alignment

   For functions with special (non-C) calling conventions:
   SYM_CODE_START -- use for non-C (special) functions
   SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o
alignment
   SYM_CODE_START_LOCAL -- use for local non-C (special) functions
   SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special)
functions, w/o alignment
   SYM_CODE_END -- the end of SYM_CODE_START_LOCAL or SYM_CODE_START

   SYM_CODE_INNER_LABEL -- only for labels in the middle of code
   SYM_CODE_INNER_LABEL_NOALIGN -- only for labels in the middle of code

d) For data
   SYM_DATA_START -- global data symbol
   SYM_DATA_END -- the end of the SYM_DATA_START symbol
   SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol
   SYM_DATA_SIMPLE -- start+end wrapper around simple global data
   SYM_DATA_SIMPLE_LOCAL -- start+end wrapper around simple local data

==

The macros allow to pair starts and ends of functions and mark functions
correctly in the output ELF objects.

All users of the old macros in x86 are converted to use these in further
patches.

thanks,
-- 
js
suse labs
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html