Re: [PATCH v5 02/11] tools: mkeficapsule: add firmwware image signing

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: Simon Glass <sjg@chromium.org>
> Date: Wed, 3 Nov 2021 20:51:25 -0600
> 
> Hi Mark,
> 
> On Tue, 2 Nov 2021 at 09:13, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Simon Glass <sjg@chromium.org>
> > > Date: Tue, 2 Nov 2021 08:56:50 -0600
> > >
> > > Hi Takahiro,
> > >
> > > > > - can we just build the tool always?
> > > >
> > > > This is one of my questions.
> > > > Why do you want to do so while there are bunch of tools that are
> > > > not always built.
> > >
> > > Because I think all tools should be built always. It is fine if that
> > > happens due to CONFIG options but we should try to avoid making it
> > > complicated.
> >
> > Well, unless this patchset fixes things, we can't, because
> > mkeficapsule doesn't build on OpenBSD.  I tried looking into it, but I
> > can't figure out how this is even supposed to compile as a host tool:
> >
> >
> > In file included from tools/mkeficapsule.c:8:
> > In file included from include/malloc.h:369:
> > include/linux/string.h:15:24: error: conflicting types for 'strspn'
> > extern __kernel_size_t strspn(const char *,const char *);
> >                        ^
> > /usr/include/string.h:88:9: note: previous declaration is here
> > size_t   strspn(const char *, const char *);
> 
> My guess is that linux/string.h should not be included, or perhaps
> __kernel_size_t should be defined to size_t.
> 
> I doubt it would take an age to figure out, with a bit of fiddling.

Well, I think the problem is quite fundamental.  Indeed I agree that
linux/string.h shouldn't be included.  It gets pulled in because the
tools include <malloc.h>.  Modern software really shouldn't include
that header anymore, and we removed it in OpenBSD some time ago.  But
even with that fixed, things break since the same header gets pulled
in from <efi.h>.

Redefining __kernel_size_t doesn't provide a way out:

tools/mkeficapsule.c:23:16: error: typedef redefinition with different types ('size_t' (aka 'unsigned long') vs 'unsigned int')
typedef size_t __kernel_size_t;
               ^
./arch/arm/include/asm/posix_types.h:37:23: note: previous definition is here
typedef unsigned int            __kernel_size_t;
	                                       ^
					       
This is on an amd64 host, so "unsigned int" clearly is the wrong type
for size_t.

The fundamental problem seems to be that <efi.h> isn't safe to include
in a "host" tool because it includes "target" headers that
accidentally resolve to "system" headers on Linux systems.

Maybe Takahiro or Heinrich have an idea how to fix that?  But in the
meantime it would be good if building this tool would remain optional.