Re: [PATCH] libebl: recognize FDO Packaging Metadata ELF note

2021-11-21 Thread Mark Wielaard
Hi Luca,

On Fri, Nov 19, 2021 at 12:31:27AM +, luca.boccassi--- via Elfutils-devel 
wrote:
> From: Luca Boccassi 
> 
> As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
> this note will be used starting from Fedora 36. Allow
> readelf --notes to pretty print it:
> 
> Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
>   Owner  Data size  Type
>   FDO   57  FDO_PACKAGING_METADATA
> Packaging Metadata: 
> {"type":"deb","name":"fsverity-utils","version":"1.3-1"}

Very nice. Thanks,

> diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
> index 36efe275..1f8bcccf 100644
> --- a/libebl/eblobjnote.c
> +++ b/libebl/eblobjnote.c
> @@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char 
> *name, uint32_t type,
>if (descsz == 0 && type == NT_VERSION)
>   return;
>  
> +  if (strcmp ("FDO", name) == 0 && type == FDO_PACKAGING_METADATA && 
> descsz > 0)
> + printf("Packaging Metadata: %.*s\n", (int) descsz, desc);
> +

We might want to check that the desc is '\0' terminated (although I
see we also don't do that in other cases, like
NT_GNU_GOLD_VERSION. But it might be good as a robustness check.

> diff --git a/libelf/elf.h b/libelf/elf.h
> index 8e3e618f..633f9f67 100644
> --- a/libelf/elf.h
> +++ b/libelf/elf.h
> @@ -1297,6 +1297,9 @@ typedef struct
>  /* Program property.  */
>  #define NT_GNU_PROPERTY_TYPE_0 5
>  
> +/* Packaging metadata as defined on 
> https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
> +#define FDO_PACKAGING_METADATA 0xcafe1a7e
> +
>  /* Note section name of program property.   */
>  #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"

Would you mind posting the elf.h patch to glibc-al...@sourceware.org.
We normally sync elf.h with the glibc one. It will also make sure
other users of elf.h also get the new constants.

As a followup I wouldn't mind a minimal testcase.
Especially if it contains a debuginfod url.

We would have to think how to integrate that with libdw
dwfl_build_id_find_elf and dwfl_standard_find_debuginfo which use
debuginfod_find from the debuginfod-client library.

Since the payload of the FDO_PACKAGING_METADATA note are not simply
key/values, but encoded in json, so we will need to add or depend on a
json parser. Any recommendations? It seems a simple enough format to
just write our own (especially if we can simply skip everything except
top-level key/value strings to find the debuginfod-url).

Thanks,

Mark



[Bug general/28608] elflint elfstrmerge fails with ld.gold

2021-11-21 Thread mark at klomp dot org via Elfutils-devel
https://sourceware.org/bugzilla/show_bug.cgi?id=28608

Mark Wielaard  changed:

   What|Removed |Added

 CC||mark at klomp dot org

--- Comment #1 from Mark Wielaard  ---
That looks like a bug in ld.gold.

tests/elfstrmerge is a simple ET_EXEC file.
When SHF_MERGE is set, then sh_entsize should be set to the size of the
elements that should be merged (unless SHF_STRING is also set).

For me .rodata doesn't have the SHF_MERGE flag set:
[15] .rodata  PROGBITS 2d68 2d68 0837  0 A 
0   0  8

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[PATCH v2] libebl: recognize FDO Packaging Metadata ELF note

2021-11-21 Thread luca.boccassi--- via Elfutils-devel
From: Luca Boccassi 

As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
this note will be used starting from Fedora 36. Allow
readelf --notes to pretty print it:

Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
  Owner  Data size  Type
  FDO   57  FDO_PACKAGING_METADATA
Packaging Metadata: {"type":"deb","name":"fsverity-utils","version":"1.3-1"}

Signed-off-by: Luca Boccassi 
---
v2: check that note payload is NULL terminated

 libebl/eblobjnote.c | 3 +++
 libebl/eblobjnotetypename.c | 3 +++
 libelf/elf.h| 3 +++
 3 files changed, 9 insertions(+)

diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
index 36efe275..e3ad1409 100644
--- a/libebl/eblobjnote.c
+++ b/libebl/eblobjnote.c
@@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char 
*name, uint32_t type,
   if (descsz == 0 && type == NT_VERSION)
return;
 
+  if (strcmp ("FDO", name) == 0 && type == FDO_PACKAGING_METADATA && 
descsz > 0 && desc[descsz - 1] == '\0')
+   printf("Packaging Metadata: %.*s\n", (int) descsz, desc);
+
   /* Everything else should have the "GNU" owner name.  */
   if (strcmp ("GNU", name) != 0)
return;
diff --git a/libebl/eblobjnotetypename.c b/libebl/eblobjnotetypename.c
index 4662906d..863f20e4 100644
--- a/libebl/eblobjnotetypename.c
+++ b/libebl/eblobjnotetypename.c
@@ -101,6 +101,9 @@ ebl_object_note_type_name (Ebl *ebl, const char *name, 
uint32_t type,
  return buf;
}
 
+  if (strcmp (name, "FDO") == 0 && type == FDO_PACKAGING_METADATA)
+   return "FDO_PACKAGING_METADATA";
+
   if (strcmp (name, "GNU") != 0)
{
  /* NT_VERSION is special, all data is in the name.  */
diff --git a/libelf/elf.h b/libelf/elf.h
index 8e3e618f..633f9f67 100644
--- a/libelf/elf.h
+++ b/libelf/elf.h
@@ -1297,6 +1297,9 @@ typedef struct
 /* Program property.  */
 #define NT_GNU_PROPERTY_TYPE_0 5
 
+/* Packaging metadata as defined on 
https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
+#define FDO_PACKAGING_METADATA 0xcafe1a7e
+
 /* Note section name of program property.   */
 #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
 
-- 
2.33.0



Re: [PATCH] libebl: recognize FDO Packaging Metadata ELF note

2021-11-21 Thread Luca Boccassi via Elfutils-devel
On Sun, 2021-11-21 at 17:33 +0100, Mark Wielaard wrote:
> Hi Luca,
> 
> On Fri, Nov 19, 2021 at 12:31:27AM +, luca.boccassi--- via
> Elfutils-devel wrote:
> > From: Luca Boccassi 
> > 
> > As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
> > this note will be used starting from Fedora 36. Allow
> > readelf --notes to pretty print it:
> > 
> > Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
> >   Owner  Data size  Type
> >   FDO   57  FDO_PACKAGING_METADATA
> >     Packaging Metadata: {"type":"deb","name":"fsverity-
> > utils","version":"1.3-1"}
> 
> Very nice. Thanks,
> 
> > diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
> > index 36efe275..1f8bcccf 100644
> > --- a/libebl/eblobjnote.c
> > +++ b/libebl/eblobjnote.c
> > @@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const
> > char *name, uint32_t type,
> >    if (descsz == 0 && type == NT_VERSION)
> > return;
> >  
> > +  if (strcmp ("FDO", name) == 0 && type ==
> > FDO_PACKAGING_METADATA && descsz > 0)
> > +   printf("    Packaging Metadata: %.*s\n", (int) descsz, desc);
> > +
> 
> We might want to check that the desc is '\0' terminated (although I
> see we also don't do that in other cases, like
> NT_GNU_GOLD_VERSION. But it might be good as a robustness check.

No problem, added in v2.

> > diff --git a/libelf/elf.h b/libelf/elf.h
> > index 8e3e618f..633f9f67 100644
> > --- a/libelf/elf.h
> > +++ b/libelf/elf.h
> > @@ -1297,6 +1297,9 @@ typedef struct
> >  /* Program property.  */
> >  #define NT_GNU_PROPERTY_TYPE_0 5
> >  
> > +/* Packaging metadata as defined on
> > https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
> > +#define FDO_PACKAGING_METADATA 0xcafe1a7e
> > +
> >  /* Note section name of program property.   */
> >  #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
> 
> Would you mind posting the elf.h patch to glibc-al...@sourceware.org.
> We normally sync elf.h with the glibc one. It will also make sure
> other users of elf.h also get the new constants.

Sure, done:

https://sourceware.org/pipermail/libc-alpha/2021-November/10.html

> As a followup I wouldn't mind a minimal testcase.
> Especially if it contains a debuginfod url.
> 
> We would have to think how to integrate that with libdw
> dwfl_build_id_find_elf and dwfl_standard_find_debuginfo which use
> debuginfod_find from the debuginfod-client library.
> 
> Since the payload of the FDO_PACKAGING_METADATA note are not simply
> key/values, but encoded in json, so we will need to add or depend on a
> json parser. Any recommendations? It seems a simple enough format to
> just write our own (especially if we can simply skip everything except
> top-level key/value strings to find the debuginfod-url).
> 
> Thanks,
> 
> Mark

Popular C parsers that I know of are json-c and jannson:

https://github.com/json-c/json-c/wiki
https://digip.org/jansson/

json-c seems to be available in slightly more places:

https://repology.org/project/json-c/versions
https://repology.org/project/jansson/versions

Rolling your own full parser can always end up being tricky and a lot
of work, especially for limited usage with no particular requirements.
You need to ensure you've got good fuzzing, etc. If using one of the
above is optional and tied to the debuginfod feature being enabled,
there shouldn't be issues with bootstrapping.
A simple search for the "debugInfoUrl" string, using whatever string is
quoted next as the url would be much simpler of course, if that's all
you need. Up to you of course.

-- 
Kind regards,
Luca Boccassi


signature.asc
Description: This is a digitally signed message part


[PATCH] debuginfod/debuginfod-client.c: correct string format on 32bit arches with 64bit time_t

2021-11-21 Thread Alexander Kanavin via Elfutils-devel
From: Alexander Kanavin 

Use intmax_t to print time_t

time_t is platform dependent and some of architectures e.g.
x32, riscv32, arc use 64bit time_t even while they are 32bit
architectures, therefore directly using integer printf formats will not
work portably, use intmax_t to typecast time_t into printf family of
functions.

musl 1.2.0 and above uses time64 on all platforms; glibc 2.34 has added support 
for
time64 (which might be enabled by distro-wide CFLAGS), with possibly
2.35 or 2.36 making it enabled by default.

Use a plain int for scanning cache_config, as that's what the function
returns.

Signed-off-by: Alexander Kanavin 
Signed-off-by: Khem Raj 
---
 debuginfod/debuginfod-client.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index c875ee62..df9737d2 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -231,15 +231,15 @@ debuginfod_config_cache(char *config_path,
   if (fd < 0)
 return -errno;
 
-  if (dprintf(fd, "%ld", cache_config_default_s) < 0)
+  if (dprintf(fd, "%jd", (intmax_t)cache_config_default_s) < 0)
 return -errno;
 }
 
-  long cache_config;
+  int cache_config;
   FILE *config_file = fopen(config_path, "r");
   if (config_file)
 {
-  if (fscanf(config_file, "%ld", &cache_config) != 1)
+  if (fscanf(config_file, "%d", &cache_config) != 1)
 cache_config = cache_config_default_s;
   fclose(config_file);
 }
@@ -272,7 +272,7 @@ debuginfod_init_cache (char *cache_path, char 
*interval_path, char *maxage_path)
   if (fd < 0)
 return -errno;
 
-  if (dprintf(fd, "%ld", cache_clean_default_interval_s) < 0)
+  if (dprintf(fd, "%jd", (intmax_t)cache_clean_default_interval_s) < 0)
 return -errno;
 
   /* init max age config file.  */
@@ -280,7 +280,7 @@ debuginfod_init_cache (char *cache_path, char 
*interval_path, char *maxage_path)
   && (fd = open(maxage_path, O_CREAT | O_RDWR, DEFFILEMODE)) < 0)
 return -errno;
 
-  if (dprintf(fd, "%ld", cache_default_max_unused_age_s) < 0)
+  if (dprintf(fd, "%jd", (intmax_t)cache_default_max_unused_age_s) < 0)
 return -errno;
 
   return 0;
-- 
2.20.1