[PATCH] ld: add --package-metadata

[PATCH] ld: add --package-metadata

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

Generate a .note.package FDO package metadata ELF note, following
the spec: https://systemd.io/COREDUMP_PACKAGE_METADATA/

If the jansson library is available at build time (and not explicitly
disabled), link ld to it, and use it to validate that the input is
correct JSON, to avoid writing garbage to the file.
If no configure option is passed and jansson (or pkg-config) is not
found, then it is just skipped (with a non-fatal warning). The
configure option --enable-jansson can be used to explicitly enable it
(error out when not found) or to explicitly disable it (don't even
look for it). This allows bootstrappers (or others who are not
interested) to seamlessly skip it without issues."
---
Now that this spec has shipped for all packages in Fedora 36, it seems
like a good time to add a specific option for it to the default linker.
Follows the same pattern of --build-id.
I've added an optional usage of libjansson to provide additional validation,
so that it doesn't have to be reimplemented everywhere a thousand times over.
For bootstrapping purposes the dep is optional and can be skipped, for example
in Debian/Ubuntu it would be annotated with <!stage1> so that everything
works out of the box.

 bfd/elf-bfd.h                        |   8 ++
 bfd/elf.c                            |   6 +-
 ld/Makefile.am                       |   6 +-
 ld/configure.ac                      |  35 ++++++++
 ld/emultempl/elf.em                  |   9 ++
 ld/ld.texi                           |   7 ++
 ld/ldelf.c                           | 121 ++++++++++++++++++++++++++-
 ld/ldelf.h                           |   2 +
 ld/lexsup.c                          |   2 +
 ld/testsuite/ld-elf/package-note.exp |  49 +++++++++++
 ld/testsuite/ld-elf/package-note.rd  |   6 ++
 11 files changed, 244 insertions(+), 7 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/package-note.exp
 create mode 100644 ld/testsuite/ld-elf/package-note.rd

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index c7c0a793b15..65bd1128263 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1918,6 +1918,14 @@ struct output_elf_obj_tdata
     asection *sec;
   } build_id;
 
+  /* FDO_PACKAGING_METADATA note type info.  */
+  struct
+  {
+    bool (*after_write_object_contents) (bfd *);
+    const char *json;
+    asection *sec;
+  } package_metadata;
+
   /* Records the result of `get_program_header_size'.  */
   bfd_size_type program_header_size;
 
diff --git a/bfd/elf.c b/bfd/elf.c
index c493aa5b172..558c630fd7b 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -6779,8 +6779,10 @@ _bfd_elf_write_object_contents (bfd *abfd)
     return false;
 
   /* This is last since write_shdrs_and_ehdr can touch i_shdrp[0].  */
-  if (t->o->build_id.after_write_object_contents != NULL)
-    return (*t->o->build_id.after_write_object_contents) (abfd);
+  if (t->o->build_id.after_write_object_contents != NULL && !(*t->o->build_id.after_write_object_contents) (abfd))
+    return false;
+  if (t->o->package_metadata.after_write_object_contents != NULL)
+    return (*t->o->package_metadata.after_write_object_contents) (abfd);
 
   return true;
 }
diff --git a/ld/Makefile.am b/ld/Makefile.am
index e53bef13bb8..a3cd7891840 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -45,7 +45,7 @@ ELF_CLFAGS=-DELF_LIST_OPTIONS=@elf_list_options@ \
 	   -DELF_PLT_UNWIND_LIST_OPTIONS=@elf_plt_unwind_list_options@
 WARN_CFLAGS = @WARN_CFLAGS@
 NO_WERROR = @NO_WERROR@
-AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS)
+AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS) $(JANSSON_CFLAGS)
 
 # We put the scripts in the directory $(scriptdir)/ldscripts.
 # We can't put the scripts in $(datadir) because the SEARCH_DIR
@@ -964,8 +964,8 @@ ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmai
 	ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \
 	ldbuildid.c
 ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \
-		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP)
-ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB)
+		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP) $(JANSSON_LIBS)
+ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB) $(JANSSON_LIBS)
 
 # Dependency tracking for the generated emulation files.
 EXTRA_ld_new_SOURCES += $(ALL_EMULATION_SOURCES) $(ALL_64_EMULATION_SOURCES)
diff --git a/ld/configure.ac b/ld/configure.ac
index 0b29e810dde..fdfb4d46344 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -287,6 +287,41 @@ fi
 AM_CONDITIONAL(ENABLE_LIBCTF, test "${enable_libctf}" = yes)
 AC_SUBST(enable_libctf)
 
+# If pkg-config and libjansson are available, use them, skip if not.
+# Allow explicitly enabling (error if not found)/disabling (skip even if found).
+AC_ARG_ENABLE([jansson],
+  [AS_HELP_STRING([--enable-jansson],
+    [enable jansson [default=auto]])],
+  [enable_jansson=$enableval],
+  [enable_jansson="auto"])
+
+if test "x$enable_jansson" != "xno"; then
+  PKG_PROG_PKG_CONFIG
+  AS_IF([test -n "$PKG_CONFIG"],
+    [
+      PKG_CHECK_MODULES(JANSSON, [jansson],
+        [
+          AC_DEFINE(HAVE_JANSSON, 1, [The jansson library is to be used])
+          AC_SUBST([JANSSON_CFLAGS])
+          AC_SUBST([JANSSON_LIBS])
+        ],
+        [
+          if test "x$enable_jansson" = "xyes"; then
+            AC_MSG_ERROR([Cannot find jansson library])
+          else
+            AC_MSG_WARN([Cannot find jansson library])
+          fi
+        ])
+    ],
+    [
+      if test "x$enable_jansson" = "xyes"; then
+        AC_MSG_ERROR([Cannot find pkg-config])
+      else
+        AC_MSG_WARN([Cannot find pkg-config])
+      fi
+    ])
+fi
+
 AM_BINUTILS_WARNINGS
 
 AM_LC_MESSAGES
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index c027559908c..c52484f35d3 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -572,6 +572,7 @@ enum elf_options
   OPTION_EXCLUDE_LIBS,
   OPTION_HASH_STYLE,
   OPTION_BUILD_ID,
+  OPTION_PACKAGE_METADATA,
   OPTION_AUDIT,
   OPTION_COMPRESS_DEBUG
 };
@@ -602,6 +603,7 @@ EOF
 fi
 fragment <<EOF
     {"build-id", optional_argument, NULL, OPTION_BUILD_ID},
+    {"package-metadata", optional_argument, NULL, OPTION_PACKAGE_METADATA},
     {"compress-debug-sections", required_argument, NULL, OPTION_COMPRESS_DEBUG},
 EOF
 if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then
@@ -650,6 +652,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
 	ldelf_emit_note_gnu_build_id = xstrdup (optarg);
       break;
 
+    case OPTION_PACKAGE_METADATA:
+      free ((char *) ldelf_emit_note_fdo_package_metadata);
+      ldelf_emit_note_fdo_package_metadata = NULL;
+      if (optarg != NULL && strlen(optarg) > 0)
+	ldelf_emit_note_fdo_package_metadata = xstrdup (optarg);
+      break;
+
     case OPTION_COMPRESS_DEBUG:
       if (strcasecmp (optarg, "none") == 0)
 	link_info.compress_debug = COMPRESS_DEBUG_NONE;
diff --git a/ld/ld.texi b/ld/ld.texi
index 8cad8478140..8406560397c 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -2935,6 +2935,13 @@ string identifying the original linked file does not change.
 
 Passing @code{none} for @var{style} disables the setting from any
 @code{--build-id} options earlier on the command line.
+
+@kindex --package-metadata=@var{JSON}
+@item --package-metadata=@var{JSON}
+Request the creation of a @code{.note.package} ELF note section.  The
+contents of the note are in JSON format, as per the package metadata
+specification.  For more information see:
+https://systemd.io/COREDUMP_PACKAGE_METADATA/
 @end table
 
 @c man end
diff --git a/ld/ldelf.c b/ld/ldelf.c
index 4094640b3f7..161fb476121 100644
--- a/ld/ldelf.c
+++ b/ld/ldelf.c
@@ -39,6 +39,9 @@
 #include <glob.h>
 #endif
 #include "ldelf.h"
+#ifdef HAVE_JANSSON
+#include <jansson.h>
+#endif
 
 struct dt_needed
 {
@@ -49,6 +52,9 @@ struct dt_needed
 /* Style of .note.gnu.build-id section.  */
 const char *ldelf_emit_note_gnu_build_id;
 
+/* Content of .note.package section.  */
+const char *ldelf_emit_note_fdo_package_metadata;
+
 /* These variables are required to pass information back and forth
    between after_open and check_needed and stat_needed and vercheck.  */
 
@@ -1249,7 +1255,8 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
 	}
     }
 
-  if (ldelf_emit_note_gnu_build_id != NULL)
+  if (ldelf_emit_note_gnu_build_id != NULL ||
+      ldelf_emit_note_fdo_package_metadata != NULL)
     {
       /* Find an ELF input.  */
       for (abfd = link_info.input_bfds;
@@ -1262,11 +1269,18 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
       /* PR 10555: If there are no ELF input files do not try to
 	 create a .note.gnu-build-id section.  */
       if (abfd == NULL
-	  || !ldelf_setup_build_id (abfd))
+	  || (ldelf_emit_note_gnu_build_id != NULL && !ldelf_setup_build_id (abfd)))
 	{
 	  free ((char *) ldelf_emit_note_gnu_build_id);
 	  ldelf_emit_note_gnu_build_id = NULL;
 	}
+
+      if (abfd == NULL
+	  || (ldelf_emit_note_fdo_package_metadata != NULL && !ldelf_setup_package_metadata (abfd)))
+	{
+	  free ((char *) ldelf_emit_note_fdo_package_metadata);
+	  ldelf_emit_note_fdo_package_metadata = NULL;
+	}
     }
 
   get_elf_backend_data (link_info.output_bfd)->setup_gnu_properties (&link_info);
@@ -1501,6 +1515,109 @@ ldelf_setup_build_id (bfd *ibfd)
   return false;
 }
 
+static bool
+write_package_metadata (bfd *abfd)
+{
+  struct elf_obj_tdata *t = elf_tdata (abfd);
+  const char *json;
+  asection *asec;
+  Elf_Internal_Shdr *i_shdr;
+  unsigned char *contents, *json_bits;
+  bfd_size_type size;
+  file_ptr position;
+  Elf_External_Note *e_note;
+
+  json = t->o->package_metadata.json;
+  asec = t->o->package_metadata.sec;
+  if (bfd_is_abs_section (asec->output_section))
+    {
+      einfo (_("%P: warning: .note.package section discarded,"
+	       " --package-metadata ignored\n"));
+      return true;
+    }
+  i_shdr = &elf_section_data (asec->output_section)->this_hdr;
+
+  if (i_shdr->contents == NULL)
+    {
+      if (asec->contents == NULL)
+	asec->contents = (unsigned char *) xmalloc (asec->size);
+      contents = asec->contents;
+    }
+  else
+    contents = i_shdr->contents + asec->output_offset;
+
+  e_note = (Elf_External_Note *) contents;
+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
+  size = (size + 3) & -(bfd_size_type) 4;
+  json_bits = contents + size;
+  size = asec->size - size;
+
+  /* Clear the package metadata field.  */
+  memset (json_bits, 0, size);
+
+  bfd_h_put_32 (abfd, sizeof "FDO", &e_note->namesz);
+  bfd_h_put_32 (abfd, size, &e_note->descsz);
+  bfd_h_put_32 (abfd, FDO_PACKAGING_METADATA, &e_note->type);
+  memcpy (e_note->name, "FDO", sizeof "FDO");
+  memcpy (json_bits, json, size);
+
+  position = i_shdr->sh_offset + asec->output_offset;
+  size = asec->size;
+  return (bfd_seek (abfd, position, SEEK_SET) == 0
+	  && bfd_bwrite (contents, size, abfd) == size);
+}
+
+/* Make .note.package section.
+   https://systemd.io/COREDUMP_PACKAGE_METADATA/  */
+
+bool
+ldelf_setup_package_metadata (bfd *ibfd)
+{
+  asection *s;
+  bfd_size_type size;
+  flagword flags;
+
+  if (!ldelf_emit_note_fdo_package_metadata || strlen(ldelf_emit_note_fdo_package_metadata) == 0)
+    return false;
+
+#ifdef HAVE_JANSSON
+  json_error_t json_error;
+  json_t *json = json_loads(ldelf_emit_note_fdo_package_metadata, 0, &json_error);
+  if (!json)
+    {
+      einfo (_("%P: warning: --package-metadata=%s does not contain valid "
+	       "JSON, ignoring: %s\n"), ldelf_emit_note_fdo_package_metadata,
+         json_error.text);
+      return false;
+    }
+  else
+    json_decref(json);
+#endif
+
+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
+  size += strlen(ldelf_emit_note_fdo_package_metadata) + 1;
+  size = (size + 3) & -(bfd_size_type) 4;
+
+  flags = (SEC_ALLOC | SEC_LOAD | SEC_IN_MEMORY
+	   | SEC_LINKER_CREATED | SEC_READONLY | SEC_DATA);
+  s = bfd_make_section_anyway_with_flags (ibfd, ".note.package",
+					  flags);
+  if (s != NULL && bfd_set_section_alignment (s, 2))
+    {
+      struct elf_obj_tdata *t = elf_tdata (link_info.output_bfd);
+      t->o->package_metadata.after_write_object_contents = &write_package_metadata;
+      t->o->package_metadata.json = ldelf_emit_note_fdo_package_metadata;
+      t->o->package_metadata.sec = s;
+      elf_section_type (s) = SHT_NOTE;
+      s->size = size;
+      return true;
+    }
+
+  einfo (_("%P: warning: cannot create .note.package section,"
+	   " --package-metadata ignored\n"));
+  return false;
+}
+
 /* Look through an expression for an assignment statement.  */
 
 static void
diff --git a/ld/ldelf.h b/ld/ldelf.h
index efa8b45851c..a3ded3dd04c 100644
--- a/ld/ldelf.h
+++ b/ld/ldelf.h
@@ -19,6 +19,7 @@
    MA 02110-1301, USA.  */
 
 extern const char *ldelf_emit_note_gnu_build_id;
+extern const char *ldelf_emit_note_fdo_package_metadata;
 
 extern void ldelf_after_parse (void);
 extern bool ldelf_load_symbols (lang_input_statement_type *);
@@ -26,6 +27,7 @@ extern void ldelf_before_plugin_all_symbols_read (int, int, int, int,
 						  int, const char *);
 extern void ldelf_after_open (int, int, int, int, int, const char *);
 extern bool ldelf_setup_build_id (bfd *);
+extern bool ldelf_setup_package_metadata (bfd *);
 extern void ldelf_append_to_separated_string (char **, char *);
 extern void ldelf_before_allocation (char *, char *, const char *);
 extern bool ldelf_open_dynamic_archive
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 78190472907..3aeba07c3a7 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2144,6 +2144,8 @@ elf_static_list_options (FILE *file)
   fprintf (file, _("\
   --build-id[=STYLE]          Generate build ID note\n"));
   fprintf (file, _("\
+  --package-metadata[=JSON]   Generate package metadata note\n"));
+  fprintf (file, _("\
   --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi]\n\
                               Compress DWARF debug sections using zlib\n"));
 #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
diff --git a/ld/testsuite/ld-elf/package-note.exp b/ld/testsuite/ld-elf/package-note.exp
new file mode 100644
index 00000000000..51a790b25de
--- /dev/null
+++ b/ld/testsuite/ld-elf/package-note.exp
@@ -0,0 +1,49 @@
+# Expect script for --package-note tests.
+#   Copyright (C) 2022 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# 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 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+# Exclude non-ELF targets.
+
+if ![is_elf_format] {
+    return
+}
+
+if { [istarget frv-*-*] || [istarget lm32-*-*] } {
+    return
+}
+
+if { !([istarget *-*-linux*]
+       || [istarget arm*-*-uclinuxfdpiceabi]
+       || [istarget *-*-nacl*]
+       || [istarget *-*-gnu*]) } then {
+    return
+}
+
+run_ld_link_tests [list \
+    [list \
+	"package-note.o" \
+	"--package-metadata='{\"foo\":\"bar\"}'" \
+	"" \
+	"" \
+	{start.s} \
+	{{readelf {--notes} package-note.rd}} \
+	"package-note.o" \
+    ] \
+]
diff --git a/ld/testsuite/ld-elf/package-note.rd b/ld/testsuite/ld-elf/package-note.rd
new file mode 100644
index 00000000000..c65dd21cdac
--- /dev/null
+++ b/ld/testsuite/ld-elf/package-note.rd
@@ -0,0 +1,6 @@
+#...
+Displaying notes found in: \.note\.package
+  Owner                Data size 	Description
+  FDO                  0x00000010	FDO_PACKAGING_METADATA
+    Packaging Metadata: {"foo":"bar"}
+#pass
-- 
2.35.1

Re: [PATCH] ld: add --package-metadata

[Fangrui Song]

On 2022-05-15, luca.boccassi--- via Binutils wrote:
>From: Luca Boccassi <bluca@debian.org>
>
>Generate a .note.package FDO package metadata ELF note, following
>the spec: https://systemd.io/COREDUMP_PACKAGE_METADATA/
>
>If the jansson library is available at build time (and not explicitly
>disabled), link ld to it, and use it to validate that the input is
>correct JSON, to avoid writing garbage to the file.
>If no configure option is passed and jansson (or pkg-config) is not
>found, then it is just skipped (with a non-fatal warning). The
>configure option --enable-jansson can be used to explicitly enable it
>(error out when not found) or to explicitly disable it (don't even
>look for it). This allows bootstrappers (or others who are not
>interested) to seamlessly skip it without issues."
>---
>Now that this spec has shipped for all packages in Fedora 36, it seems
>like a good time to add a specific option for it to the default linker.
>Follows the same pattern of --build-id.

Both the "FDO" and the json dependency make me concerned -
if a linker script approach works quite well, why bother with a new
linker option with a very specific application?

--build-id is a bit different in that it adds a section whose content is
computed by the linker. I am not sure having --build-id implies
the necessity of --package-metadata.

>I've added an optional usage of libjansson to provide additional validation,
>so that it doesn't have to be reimplemented everywhere a thousand times over.
>For bootstrapping purposes the dep is optional and can be skipped, for example
>in Debian/Ubuntu it would be annotated with <!stage1> so that everything
>works out of the box.
>
> bfd/elf-bfd.h                        |   8 ++
> bfd/elf.c                            |   6 +-
> ld/Makefile.am                       |   6 +-
> ld/configure.ac                      |  35 ++++++++
> ld/emultempl/elf.em                  |   9 ++
> ld/ld.texi                           |   7 ++
> ld/ldelf.c                           | 121 ++++++++++++++++++++++++++-
> ld/ldelf.h                           |   2 +
> ld/lexsup.c                          |   2 +
> ld/testsuite/ld-elf/package-note.exp |  49 +++++++++++
> ld/testsuite/ld-elf/package-note.rd  |   6 ++
> 11 files changed, 244 insertions(+), 7 deletions(-)
> create mode 100644 ld/testsuite/ld-elf/package-note.exp
> create mode 100644 ld/testsuite/ld-elf/package-note.rd
>
>diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
>index c7c0a793b15..65bd1128263 100644
>--- a/bfd/elf-bfd.h
>+++ b/bfd/elf-bfd.h
>@@ -1918,6 +1918,14 @@ struct output_elf_obj_tdata
>     asection *sec;
>   } build_id;
>
>+  /* FDO_PACKAGING_METADATA note type info.  */
>+  struct
>+  {
>+    bool (*after_write_object_contents) (bfd *);
>+    const char *json;
>+    asection *sec;
>+  } package_metadata;
>+
>   /* Records the result of `get_program_header_size'.  */
>   bfd_size_type program_header_size;
>
>diff --git a/bfd/elf.c b/bfd/elf.c
>index c493aa5b172..558c630fd7b 100644
>--- a/bfd/elf.c
>+++ b/bfd/elf.c
>@@ -6779,8 +6779,10 @@ _bfd_elf_write_object_contents (bfd *abfd)
>     return false;
>
>   /* This is last since write_shdrs_and_ehdr can touch i_shdrp[0].  */
>-  if (t->o->build_id.after_write_object_contents != NULL)
>-    return (*t->o->build_id.after_write_object_contents) (abfd);
>+  if (t->o->build_id.after_write_object_contents != NULL && !(*t->o->build_id.after_write_object_contents) (abfd))
>+    return false;
>+  if (t->o->package_metadata.after_write_object_contents != NULL)
>+    return (*t->o->package_metadata.after_write_object_contents) (abfd);
>
>   return true;
> }
>diff --git a/ld/Makefile.am b/ld/Makefile.am
>index e53bef13bb8..a3cd7891840 100644
>--- a/ld/Makefile.am
>+++ b/ld/Makefile.am
>@@ -45,7 +45,7 @@ ELF_CLFAGS=-DELF_LIST_OPTIONS=@elf_list_options@ \
> 	   -DELF_PLT_UNWIND_LIST_OPTIONS=@elf_plt_unwind_list_options@
> WARN_CFLAGS = @WARN_CFLAGS@
> NO_WERROR = @NO_WERROR@
>-AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS)
>+AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS) $(JANSSON_CFLAGS)
>
> # We put the scripts in the directory $(scriptdir)/ldscripts.
> # We can't put the scripts in $(datadir) because the SEARCH_DIR
>@@ -964,8 +964,8 @@ ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmai
> 	ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \
> 	ldbuildid.c
> ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \
>-		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP)
>-ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB)
>+		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP) $(JANSSON_LIBS)
>+ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB) $(JANSSON_LIBS)
>
> # Dependency tracking for the generated emulation files.
> EXTRA_ld_new_SOURCES += $(ALL_EMULATION_SOURCES) $(ALL_64_EMULATION_SOURCES)
>diff --git a/ld/configure.ac b/ld/configure.ac
>index 0b29e810dde..fdfb4d46344 100644
>--- a/ld/configure.ac
>+++ b/ld/configure.ac
>@@ -287,6 +287,41 @@ fi
> AM_CONDITIONAL(ENABLE_LIBCTF, test "${enable_libctf}" = yes)
> AC_SUBST(enable_libctf)
>
>+# If pkg-config and libjansson are available, use them, skip if not.
>+# Allow explicitly enabling (error if not found)/disabling (skip even if found).
>+AC_ARG_ENABLE([jansson],
>+  [AS_HELP_STRING([--enable-jansson],
>+    [enable jansson [default=auto]])],
>+  [enable_jansson=$enableval],
>+  [enable_jansson="auto"])
>+
>+if test "x$enable_jansson" != "xno"; then
>+  PKG_PROG_PKG_CONFIG
>+  AS_IF([test -n "$PKG_CONFIG"],
>+    [
>+      PKG_CHECK_MODULES(JANSSON, [jansson],
>+        [
>+          AC_DEFINE(HAVE_JANSSON, 1, [The jansson library is to be used])
>+          AC_SUBST([JANSSON_CFLAGS])
>+          AC_SUBST([JANSSON_LIBS])
>+        ],
>+        [
>+          if test "x$enable_jansson" = "xyes"; then
>+            AC_MSG_ERROR([Cannot find jansson library])
>+          else
>+            AC_MSG_WARN([Cannot find jansson library])
>+          fi
>+        ])
>+    ],
>+    [
>+      if test "x$enable_jansson" = "xyes"; then
>+        AC_MSG_ERROR([Cannot find pkg-config])
>+      else
>+        AC_MSG_WARN([Cannot find pkg-config])
>+      fi
>+    ])
>+fi
>+
> AM_BINUTILS_WARNINGS
>
> AM_LC_MESSAGES
>diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
>index c027559908c..c52484f35d3 100644
>--- a/ld/emultempl/elf.em
>+++ b/ld/emultempl/elf.em
>@@ -572,6 +572,7 @@ enum elf_options
>   OPTION_EXCLUDE_LIBS,
>   OPTION_HASH_STYLE,
>   OPTION_BUILD_ID,
>+  OPTION_PACKAGE_METADATA,
>   OPTION_AUDIT,
>   OPTION_COMPRESS_DEBUG
> };
>@@ -602,6 +603,7 @@ EOF
> fi
> fragment <<EOF
>     {"build-id", optional_argument, NULL, OPTION_BUILD_ID},
>+    {"package-metadata", optional_argument, NULL, OPTION_PACKAGE_METADATA},
>     {"compress-debug-sections", required_argument, NULL, OPTION_COMPRESS_DEBUG},
> EOF
> if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then
>@@ -650,6 +652,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
> 	ldelf_emit_note_gnu_build_id = xstrdup (optarg);
>       break;
>
>+    case OPTION_PACKAGE_METADATA:
>+      free ((char *) ldelf_emit_note_fdo_package_metadata);
>+      ldelf_emit_note_fdo_package_metadata = NULL;
>+      if (optarg != NULL && strlen(optarg) > 0)
>+	ldelf_emit_note_fdo_package_metadata = xstrdup (optarg);
>+      break;
>+
>     case OPTION_COMPRESS_DEBUG:
>       if (strcasecmp (optarg, "none") == 0)
> 	link_info.compress_debug = COMPRESS_DEBUG_NONE;
>diff --git a/ld/ld.texi b/ld/ld.texi
>index 8cad8478140..8406560397c 100644
>--- a/ld/ld.texi
>+++ b/ld/ld.texi
>@@ -2935,6 +2935,13 @@ string identifying the original linked file does not change.
>
> Passing @code{none} for @var{style} disables the setting from any
> @code{--build-id} options earlier on the command line.
>+
>+@kindex --package-metadata=@var{JSON}
>+@item --package-metadata=@var{JSON}
>+Request the creation of a @code{.note.package} ELF note section.  The
>+contents of the note are in JSON format, as per the package metadata
>+specification.  For more information see:
>+https://systemd.io/COREDUMP_PACKAGE_METADATA/
> @end table
>
> @c man end
>diff --git a/ld/ldelf.c b/ld/ldelf.c
>index 4094640b3f7..161fb476121 100644
>--- a/ld/ldelf.c
>+++ b/ld/ldelf.c
>@@ -39,6 +39,9 @@
> #include <glob.h>
> #endif
> #include "ldelf.h"
>+#ifdef HAVE_JANSSON
>+#include <jansson.h>
>+#endif
>
> struct dt_needed
> {
>@@ -49,6 +52,9 @@ struct dt_needed
> /* Style of .note.gnu.build-id section.  */
> const char *ldelf_emit_note_gnu_build_id;
>
>+/* Content of .note.package section.  */
>+const char *ldelf_emit_note_fdo_package_metadata;
>+
> /* These variables are required to pass information back and forth
>    between after_open and check_needed and stat_needed and vercheck.  */
>
>@@ -1249,7 +1255,8 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
> 	}
>     }
>
>-  if (ldelf_emit_note_gnu_build_id != NULL)
>+  if (ldelf_emit_note_gnu_build_id != NULL ||
>+      ldelf_emit_note_fdo_package_metadata != NULL)
>     {
>       /* Find an ELF input.  */
>       for (abfd = link_info.input_bfds;
>@@ -1262,11 +1269,18 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
>       /* PR 10555: If there are no ELF input files do not try to
> 	 create a .note.gnu-build-id section.  */
>       if (abfd == NULL
>-	  || !ldelf_setup_build_id (abfd))
>+	  || (ldelf_emit_note_gnu_build_id != NULL && !ldelf_setup_build_id (abfd)))
> 	{
> 	  free ((char *) ldelf_emit_note_gnu_build_id);
> 	  ldelf_emit_note_gnu_build_id = NULL;
> 	}
>+
>+      if (abfd == NULL
>+	  || (ldelf_emit_note_fdo_package_metadata != NULL && !ldelf_setup_package_metadata (abfd)))
>+	{
>+	  free ((char *) ldelf_emit_note_fdo_package_metadata);
>+	  ldelf_emit_note_fdo_package_metadata = NULL;
>+	}
>     }
>
>   get_elf_backend_data (link_info.output_bfd)->setup_gnu_properties (&link_info);
>@@ -1501,6 +1515,109 @@ ldelf_setup_build_id (bfd *ibfd)
>   return false;
> }
>
>+static bool
>+write_package_metadata (bfd *abfd)
>+{
>+  struct elf_obj_tdata *t = elf_tdata (abfd);
>+  const char *json;
>+  asection *asec;
>+  Elf_Internal_Shdr *i_shdr;
>+  unsigned char *contents, *json_bits;
>+  bfd_size_type size;
>+  file_ptr position;
>+  Elf_External_Note *e_note;
>+
>+  json = t->o->package_metadata.json;
>+  asec = t->o->package_metadata.sec;
>+  if (bfd_is_abs_section (asec->output_section))
>+    {
>+      einfo (_("%P: warning: .note.package section discarded,"
>+	       " --package-metadata ignored\n"));
>+      return true;
>+    }
>+  i_shdr = &elf_section_data (asec->output_section)->this_hdr;
>+
>+  if (i_shdr->contents == NULL)
>+    {
>+      if (asec->contents == NULL)
>+	asec->contents = (unsigned char *) xmalloc (asec->size);
>+      contents = asec->contents;
>+    }
>+  else
>+    contents = i_shdr->contents + asec->output_offset;
>+
>+  e_note = (Elf_External_Note *) contents;
>+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
>+  size = (size + 3) & -(bfd_size_type) 4;
>+  json_bits = contents + size;
>+  size = asec->size - size;
>+
>+  /* Clear the package metadata field.  */
>+  memset (json_bits, 0, size);
>+
>+  bfd_h_put_32 (abfd, sizeof "FDO", &e_note->namesz);
>+  bfd_h_put_32 (abfd, size, &e_note->descsz);
>+  bfd_h_put_32 (abfd, FDO_PACKAGING_METADATA, &e_note->type);
>+  memcpy (e_note->name, "FDO", sizeof "FDO");
>+  memcpy (json_bits, json, size);
>+
>+  position = i_shdr->sh_offset + asec->output_offset;
>+  size = asec->size;
>+  return (bfd_seek (abfd, position, SEEK_SET) == 0
>+	  && bfd_bwrite (contents, size, abfd) == size);
>+}
>+
>+/* Make .note.package section.
>+   https://systemd.io/COREDUMP_PACKAGE_METADATA/  */
>+
>+bool
>+ldelf_setup_package_metadata (bfd *ibfd)
>+{
>+  asection *s;
>+  bfd_size_type size;
>+  flagword flags;
>+
>+  if (!ldelf_emit_note_fdo_package_metadata || strlen(ldelf_emit_note_fdo_package_metadata) == 0)
>+    return false;
>+
>+#ifdef HAVE_JANSSON
>+  json_error_t json_error;
>+  json_t *json = json_loads(ldelf_emit_note_fdo_package_metadata, 0, &json_error);
>+  if (!json)
>+    {
>+      einfo (_("%P: warning: --package-metadata=%s does not contain valid "
>+	       "JSON, ignoring: %s\n"), ldelf_emit_note_fdo_package_metadata,
>+         json_error.text);
>+      return false;
>+    }
>+  else
>+    json_decref(json);
>+#endif
>+
>+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
>+  size += strlen(ldelf_emit_note_fdo_package_metadata) + 1;
>+  size = (size + 3) & -(bfd_size_type) 4;
>+
>+  flags = (SEC_ALLOC | SEC_LOAD | SEC_IN_MEMORY
>+	   | SEC_LINKER_CREATED | SEC_READONLY | SEC_DATA);
>+  s = bfd_make_section_anyway_with_flags (ibfd, ".note.package",
>+					  flags);
>+  if (s != NULL && bfd_set_section_alignment (s, 2))
>+    {
>+      struct elf_obj_tdata *t = elf_tdata (link_info.output_bfd);
>+      t->o->package_metadata.after_write_object_contents = &write_package_metadata;
>+      t->o->package_metadata.json = ldelf_emit_note_fdo_package_metadata;
>+      t->o->package_metadata.sec = s;
>+      elf_section_type (s) = SHT_NOTE;
>+      s->size = size;
>+      return true;
>+    }
>+
>+  einfo (_("%P: warning: cannot create .note.package section,"
>+	   " --package-metadata ignored\n"));
>+  return false;
>+}
>+
> /* Look through an expression for an assignment statement.  */
>
> static void
>diff --git a/ld/ldelf.h b/ld/ldelf.h
>index efa8b45851c..a3ded3dd04c 100644
>--- a/ld/ldelf.h
>+++ b/ld/ldelf.h
>@@ -19,6 +19,7 @@
>    MA 02110-1301, USA.  */
>
> extern const char *ldelf_emit_note_gnu_build_id;
>+extern const char *ldelf_emit_note_fdo_package_metadata;
>
> extern void ldelf_after_parse (void);
> extern bool ldelf_load_symbols (lang_input_statement_type *);
>@@ -26,6 +27,7 @@ extern void ldelf_before_plugin_all_symbols_read (int, int, int, int,
> 						  int, const char *);
> extern void ldelf_after_open (int, int, int, int, int, const char *);
> extern bool ldelf_setup_build_id (bfd *);
>+extern bool ldelf_setup_package_metadata (bfd *);
> extern void ldelf_append_to_separated_string (char **, char *);
> extern void ldelf_before_allocation (char *, char *, const char *);
> extern bool ldelf_open_dynamic_archive
>diff --git a/ld/lexsup.c b/ld/lexsup.c
>index 78190472907..3aeba07c3a7 100644
>--- a/ld/lexsup.c
>+++ b/ld/lexsup.c
>@@ -2144,6 +2144,8 @@ elf_static_list_options (FILE *file)
>   fprintf (file, _("\
>   --build-id[=STYLE]          Generate build ID note\n"));
>   fprintf (file, _("\
>+  --package-metadata[=JSON]   Generate package metadata note\n"));
>+  fprintf (file, _("\
>   --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi]\n\
>                               Compress DWARF debug sections using zlib\n"));
> #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
>diff --git a/ld/testsuite/ld-elf/package-note.exp b/ld/testsuite/ld-elf/package-note.exp
>new file mode 100644
>index 00000000000..51a790b25de
>--- /dev/null
>+++ b/ld/testsuite/ld-elf/package-note.exp
>@@ -0,0 +1,49 @@
>+# Expect script for --package-note tests.
>+#   Copyright (C) 2022 Free Software Foundation, Inc.
>+#
>+# This file is part of the GNU Binutils.
>+#
>+# 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 3 of the License, or
>+# (at your option) any later version.
>+#
>+# This program is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with this program; if not, write to the Free Software
>+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
>+# MA 02110-1301, USA.
>+#
>+
>+# Exclude non-ELF targets.
>+
>+if ![is_elf_format] {
>+    return
>+}
>+
>+if { [istarget frv-*-*] || [istarget lm32-*-*] } {
>+    return
>+}
>+
>+if { !([istarget *-*-linux*]
>+       || [istarget arm*-*-uclinuxfdpiceabi]
>+       || [istarget *-*-nacl*]
>+       || [istarget *-*-gnu*]) } then {
>+    return
>+}
>+
>+run_ld_link_tests [list \
>+    [list \
>+	"package-note.o" \
>+	"--package-metadata='{\"foo\":\"bar\"}'" \
>+	"" \
>+	"" \
>+	{start.s} \
>+	{{readelf {--notes} package-note.rd}} \
>+	"package-note.o" \
>+    ] \
>+]
>diff --git a/ld/testsuite/ld-elf/package-note.rd b/ld/testsuite/ld-elf/package-note.rd
>new file mode 100644
>index 00000000000..c65dd21cdac
>--- /dev/null
>+++ b/ld/testsuite/ld-elf/package-note.rd
>@@ -0,0 +1,6 @@
>+#...
>+Displaying notes found in: \.note\.package
>+  Owner                Data size 	Description
>+  FDO                  0x00000010	FDO_PACKAGING_METADATA
>+    Packaging Metadata: {"foo":"bar"}
>+#pass
>-- 
>2.35.1
>

Re: [PATCH] ld: add --package-metadata

[Florian Weimer]

* Fangrui Song:

> Both the "FDO" and the json dependency make me concerned -
> if a linker script approach works quite well, why bother with a new
> linker option with a very specific application?

The linker script does not work all that well because it requires that
the script is materialized to disk somewhere, and that path needs to
show up in the build flags.  If the build flags are stored beyond the
build for future use, this makes them invalid because the path will
typically be gone by the time the stored flags are used.

Thanks,
Florian

Re: [PATCH] ld: add --package-metadata

[Luca Boccassi]

On Tue, 17 May 2022 at 07:03, Florian Weimer <fweimer@redhat.com> wrote:
>
> * Fangrui Song:
>
> > Both the "FDO" and the json dependency make me concerned -
> > if a linker script approach works quite well, why bother with a new
> > linker option with a very specific application?
>
> The linker script does not work all that well because it requires that
> the script is materialized to disk somewhere, and that path needs to
> show up in the build flags.  If the build flags are stored beyond the
> build for future use, this makes them invalid because the path will
> typically be gone by the time the stored flags are used.
>
> Thanks,
> Florian

Indeed, it was working fine as an initial step to get the ball rolling and
the spec adopted, but having to generate and reference an
intermediate, temporary asset is not great and has drawbacks. Now
that the spec is live and proven in production in Fedora 36 it seemed
to me like a good time to introduce a first-class option, to make further
usage and adoption as trivial as it can be.

Kind regards,
Luca Boccassi

Re: [PATCH] ld: add --package-metadata

[Luca Boccassi]

On Sun, 15 May 2022 at 20:21, <luca.boccassi@gmail.com> wrote:
>
> From: Luca Boccassi <bluca@debian.org>
>
> Generate a .note.package FDO package metadata ELF note, following
> the spec: https://systemd.io/COREDUMP_PACKAGE_METADATA/
>
> If the jansson library is available at build time (and not explicitly
> disabled), link ld to it, and use it to validate that the input is
> correct JSON, to avoid writing garbage to the file.
> If no configure option is passed and jansson (or pkg-config) is not
> found, then it is just skipped (with a non-fatal warning). The
> configure option --enable-jansson can be used to explicitly enable it
> (error out when not found) or to explicitly disable it (don't even
> look for it). This allows bootstrappers (or others who are not
> interested) to seamlessly skip it without issues."
> ---
> Now that this spec has shipped for all packages in Fedora 36, it seems
> like a good time to add a specific option for it to the default linker.
> Follows the same pattern of --build-id.
> I've added an optional usage of libjansson to provide additional validation,
> so that it doesn't have to be reimplemented everywhere a thousand times over.
> For bootstrapping purposes the dep is optional and can be skipped, for example
> in Debian/Ubuntu it would be annotated with <!stage1> so that everything
> works out of the box.
>
>  bfd/elf-bfd.h                        |   8 ++
>  bfd/elf.c                            |   6 +-
>  ld/Makefile.am                       |   6 +-
>  ld/configure.ac                      |  35 ++++++++
>  ld/emultempl/elf.em                  |   9 ++
>  ld/ld.texi                           |   7 ++
>  ld/ldelf.c                           | 121 ++++++++++++++++++++++++++-
>  ld/ldelf.h                           |   2 +
>  ld/lexsup.c                          |   2 +
>  ld/testsuite/ld-elf/package-note.exp |  49 +++++++++++
>  ld/testsuite/ld-elf/package-note.rd  |   6 ++
>  11 files changed, 244 insertions(+), 7 deletions(-)
>  create mode 100644 ld/testsuite/ld-elf/package-note.exp
>  create mode 100644 ld/testsuite/ld-elf/package-note.rd

Hi Nick,

Did you have any chance to look at this? Any thoughts? Thanks!

Kind regards,
Luca Boccassi

Re: [PATCH] ld: add --package-metadata

[Jose E. Marchesi]

Hi Luca.

>> I've added an optional usage of libjansson to provide additional validation,
>> so that it doesn't have to be reimplemented everywhere a thousand
>> times over.
>> For bootstrapping purposes the dep is optional and can be skipped, for example
>> in Debian/Ubuntu it would be annotated with <!stage1> so that everything
>> works out of the box.

As far as I can see at https://systemd.io/COREDUMP_PACKAGE_METADATA/ the
payload encoded in JSON comprises a single object which must be a
dictionary, whose entries all contain strings.  Basically:

  {"..." : "..."[, "..." : "..."]*}

Isn't it a bit overkill to add a dependency to an external library just
to verify the format of the above?  A regular expression would suffice,
even if a fastidious one due to the string format:

  S -> JSON string
  B -> [\n\t ]*

  PAYLOAD -> B{BSB:BSB(,SB:BSB)*}B

>>
>>  bfd/elf-bfd.h                        |   8 ++
>>  bfd/elf.c                            |   6 +-
>>  ld/Makefile.am                       |   6 +-
>>  ld/configure.ac                      |  35 ++++++++
>>  ld/emultempl/elf.em                  |   9 ++
>>  ld/ld.texi                           |   7 ++
>>  ld/ldelf.c                           | 121 ++++++++++++++++++++++++++-
>>  ld/ldelf.h                           |   2 +
>>  ld/lexsup.c                          |   2 +
>>  ld/testsuite/ld-elf/package-note.exp |  49 +++++++++++
>>  ld/testsuite/ld-elf/package-note.rd  |   6 ++
>>  11 files changed, 244 insertions(+), 7 deletions(-)
>>  create mode 100644 ld/testsuite/ld-elf/package-note.exp
>>  create mode 100644 ld/testsuite/ld-elf/package-note.rd
>
> Hi Nick,
>
> Did you have any chance to look at this? Any thoughts? Thanks!
>
> Kind regards,
> Luca Boccassi

Re: [PATCH] ld: add --package-metadata

[Luca Boccassi]

On Tue, 24 May 2022 at 12:13, Jose E. Marchesi <jose.marchesi@oracle.com> wrote:
>
>
> Hi Luca.
>
> >> I've added an optional usage of libjansson to provide additional validation,
> >> so that it doesn't have to be reimplemented everywhere a thousand
> >> times over.
> >> For bootstrapping purposes the dep is optional and can be skipped, for example
> >> in Debian/Ubuntu it would be annotated with <!stage1> so that everything
> >> works out of the box.
>
> As far as I can see at https://systemd.io/COREDUMP_PACKAGE_METADATA/ the
> payload encoded in JSON comprises a single object which must be a
> dictionary, whose entries all contain strings.  Basically:
>
>   {"..." : "..."[, "..." : "..."]*}
>
> Isn't it a bit overkill to add a dependency to an external library just
> to verify the format of the above?  A regular expression would suffice,
> even if a fastidious one due to the string format:
>
>   S -> JSON string
>   B -> [\n\t ]*
>
>   PAYLOAD -> B{BSB:BSB(,SB:BSB)*}B

Hi,

The specification actually is very explicit in supporting a full json
payload, with the only limitations (certain unicode characters, int
ranges) being explicitly called out. The example given is simple
because it's just an example. I do expect the usage to expand at some
point in the future.

As mentioned in the patch the validation is entirely optional, and
nothing will happen if the build dependency isn't provided, which will
be the default in all distros. And it can also be explicitly disabled
if desired. I could flip the default to force-disable if desired, but
in practice I don't think it would make much difference, given how
packages are built in minimal environments already.

As a distribution builder I would very very much like to find out
about build-time issues at build time, rather than at runtime. It's
much cheaper and immediate. When you have 50000+ packages it is very
much unrealistic to expect to runtime QA them all for every little
thing like this. An automated, implicit build time check is so much
cheaper, reliable and effective, and reduces the need to reimplement
it every single time.

Kind regards,
Luca Boccassi

Re: [PATCH] ld: add --package-metadata

[Nick Clifton]

Hi Luca,

   First of all a couple of questions about the need for the patch itself.

   Firstly - why modify the linker itself ?  Couldn't the same effect be
   achieved by using the assembler to create an object file and then
   including that in the link along with the other objects ?

   Secondly - why build json verification into the linker ?  It does not
   feel like a linkery kind of thing to do, and because the verification
   is dependent upon the linker being configured and built with the feature
   enabled, it means that programmers cannot rely upon their packaging
   notes always being checked.  To me, I think that it would be better to
   have a pre- or post- links stage where the note is checked by a separate
   tool.

   Lastly, since this option is being used to create a note section, maybe
   we ought to consider the need for a generic mechanism for the linker to
   create sections - other than using linker scripts.  For example, suppose
   that instead of --package-metadata=<JSON> there was an option:
   --add-note=<SEC-NAME>,<NOTE-NAME>,<CONTENTS>,[<VERIFIER>]
   which would basically do the same thing, but in a more generic fashion.

   Some thoughts on the patch itself:

> +  if (t->o->build_id.after_write_object_contents != NULL && !(*t->o->build_id.after_write_object_contents) (abfd))
> +    return false;

Line length - the if-statement should be over two lines.

> +  if (t->o->package_metadata.after_write_object_contents != NULL)
> +    return (*t->o->package_metadata.after_write_object_contents) (abfd);

For consistency sake, it would be nice if these two lines
were turned into a two predicate if-statement like the ones
above.  That way if other after_write_xxx functions are added
in the future, then this if-statement will not need to be
modified.

> +@kindex --package-metadata=@var{JSON}
> +@item --package-metadata=@var{JSON}
> +Request the creation of a @code{.note.package} ELF note section.  The
> +contents of the note are in JSON format, as per the package metadata
> +specification.  For more information see:
> +https://systemd.io/COREDUMP_PACKAGE_METADATA/

You should mention that if the JSON argument is missing/empty then this
will disable the creation of the metadata note, if one had been enabled
by an earlier occurrence of the --package-metdata option.

You should also add an entry to the ld/NEWS file about this new feature.

It might also be worth mentioning that the contents of the JSON text
will be checked for conformance to the standard, if the linker has been
configured and built that way.

Also - can the JSON data be read from a file ?  I can easily see problems
being encountered with the length of the linker command line if the JSON
data can only be specified as text.  (Possibly this can be avoided by using
the @file syntax already supported by the linker).

> -  if (ldelf_emit_note_gnu_build_id != NULL)
> +  if (ldelf_emit_note_gnu_build_id != NULL ||
> +      ldelf_emit_note_fdo_package_metadata != NULL)

Formatting - the || should be at the start of the next line.

> +  if (!ldelf_emit_note_fdo_package_metadata || strlen(ldelf_emit_note_fdo_package_metadata) == 0)
> +    return false;

Formatting - a space between strlen and the opening parenthesis please.

> +  json_t *json = json_loads(ldelf_emit_note_fdo_package_metadata, 0, &json_error);
> +    json_decref(json);

Likewise for these two function calls.


> +if { [istarget frv-*-*] || [istarget lm32-*-*] } {
> +    return
> +}

Why are these architectures being skipped ?

Cheers
   Nick

Re: [PATCH] ld: add --package-metadata

[Nick Clifton]

Hi Luca,

   I have just discovered another problem with the patch.  Compiling
   a toolchain with address sanitization enabled and then running the
   package-note.o test results in:

==2181020==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000007e at pc 0x147930f35e0b bp 0x7fff7bbf1820 sp 0x7fff7bbf0fd0
READ of size 16 at 0x60200000007e thread T0
     #0 0x147930f35e0a in __interceptor_memcpy (/lib64/libasan.so.8+0x49e0a)
     #1 0x8a17df in write_package_metadata /work/sources/binutils/current/ld/ldelf.c:1562
     #2 0x98e92d in _bfd_elf_write_object_contents /work/sources/binutils/current/bfd/elf.c:6790

0x60200000007e is located 0 bytes to the right of 14-byte region [0x602000000070,0x60200000007e)
allocated by thread T0 here:
     #0 0x147930fa668f in __interceptor_malloc (/lib64/libasan.so.8+0xba68f)
     #1 0x1642823 in xmalloc /work/sources/binutils/current/libiberty/xmalloc.c:149
     #2 0x1642a13 in xstrdup /work/sources/binutils/current/libiberty/xstrdup.c:34
     #3 0x85a988 in gldelf_x86_64_handle_option /work/builds/binutils/current/sanitize-address/ld/eelf_x86_64.c:5361

I think that you might be trying to write out too many bytes....

Cheers
   Nick

Re: [PATCH] ld: add --package-metadata

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 4809 bytes --]

On Tue, 2022-05-24 at 17:23 +0100, Nick Clifton wrote:
> Hi Luca,
> 
>    First of all a couple of questions about the need for the patch itself.
> 
>    Firstly - why modify the linker itself ?  Couldn't the same effect be
>    achieved by using the assembler to create an object file and then
>    including that in the link along with the other objects ?

So we are already using an intermediary artifact, and it worked well to
quickly get a workable PoC together, and then hammer it in shape to
deploy it distro-wide in Fedora and CBL-Mariner. Now that we are in
production phase, and we'de like for the spec to expand (and take over
the world!), we want to address an important shortcoming that affects
distro builders.

Dealing with intermediate artifacts means dealing with paths. And in
practice, that gets really, really, really awkward and problematic.
Almost all issues we've had are due to the fact that being able to
reliably reference a path at build time (while not breaking
reproducibility!) is _extremely_ complicated, and there's a huge tail
end of packages were things are just too hopelessly convoluted. It
might sound counter-intuitive as it's so trivial to put together an
hello world, but in reality when you are dealing with 20k to 30k
packages things get really, really weird. Coverage in F36 is not 100%
of binaries mostly because of this.

So with that experience in the bag, the most obvious next step is to
have a first-class option, that allows a self-contained flag to be
passed instead. After all the idea is similar to the build-id, and
there we have a first-class option too, and it works well.

>    Secondly - why build json verification into the linker ?  It does not
>    feel like a linkery kind of thing to do, and because the verification
>    is dependent upon the linker being configured and built with the feature
>    enabled, it means that programmers cannot rely upon their packaging
>    notes always being checked.  To me, I think that it would be better to
>    have a pre- or post- links stage where the note is checked by a separate
>    tool.

This is not aimed at individual programmers - an individual developer
is not expected to worry about any of this when hacking on local
software and such. The spec is made for distribution builders. And
speaking as one, my life would be so much easier if input was validated
before usage. Leaving it as an exercise for the reader means that
_every single distribution builder_ would need to reimplement the exact
same check, over and over and over again. Get it done once and everyone
can benefit. JSON looks the same across all distros, after all!

Or from another point of view: ld already validates other types of
input. Linker scripts get validated. Flags get validated. Objects get
validated. The type of build-id requested gets validated. So why not
allow (optionally!) to validate this too? It's easy, just half a dozen
lines of code, if the lib is missing nothing happens and it's just
skipped.

If there are any concerns I can flip the autoconf to be default-
disabled, so that --enable has to be passed explicitly, together with
making the dep available at build time?

>    Lastly, since this option is being used to create a note section, maybe
>    we ought to consider the need for a generic mechanism for the linker to
>    create sections - other than using linker scripts.  For example, suppose
>    that instead of --package-metadata=<JSON> there was an option:
>    --add-note=<SEC-NAME>,<NOTE-NAME>,<CONTENTS>,[<VERIFIER>]
>    which would basically do the same thing, but in a more generic fashion.

Well we would lose verification, and also it sounds quite a bit more
complex (you'd need to pass IDs in hex format - quite a bit ugly) - if
there hasn't been a need so far, not sure it's worth it?

>    Some thoughts on the patch itself:

<..>

> Also - can the JSON data be read from a file ?  I can easily see problems
> being encountered with the length of the linker command line if the JSON
> data can only be specified as text.  (Possibly this can be avoided by using
> the @file syntax already supported by the linker).

It could be a separate option I guess, if needed, but we wouldn't use
it - as mentioned above, the main goal is to get rid of the
intermediate file entirely.

<..>

> 
> > +if { [istarget frv-*-*] || [istarget lm32-*-*] } {
> > +    return
> > +}
> 
> Why are these architectures being skipped ?

To be perfectly honest - cargo culting from another test :-) Removed
now.

Will send a v2 later with fixes for all the style issues and the ASAN
one.

Thanks for the review!


-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2] ld: add --package-metadata

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

Generate a .note.package FDO package metadata ELF note, following
the spec: https://systemd.io/ELF_PACKAGE_METADATA/

If the jansson library is available at build time (and not explicitly
disabled), link ld to it, and use it to validate that the input is
correct JSON, to avoid writing garbage to the file.
If no configure option is passed and jansson (or pkg-config) is not
found, then it is just skipped (with a non-fatal warning). The
configure option --enable-jansson can be used to explicitly enable it
(error out when not found) or to explicitly disable it (don't even
look for it). This allows bootstrappers (or others who are not
interested) to seamlessly skip it without issues."
---
v2: fix style issues
    fix ASAN issue: note storage is rounded up, but that size
    was used to read the json input, which is fixed
    add entry to NEWS
    add note about json validation to docs

 bfd/elf-bfd.h                        |   8 ++
 bfd/elf.c                            |   8 +-
 ld/Makefile.am                       |   6 +-
 ld/NEWS                              |   6 ++
 ld/configure.ac                      |  35 +++++++
 ld/emultempl/elf.em                  |   9 ++
 ld/ld.texi                           |  12 +++
 ld/ldelf.c                           | 136 ++++++++++++++++++++++++++-
 ld/ldelf.h                           |   2 +
 ld/lexsup.c                          |   2 +
 ld/testsuite/ld-elf/package-note.exp |  45 +++++++++
 ld/testsuite/ld-elf/package-note.rd  |   6 ++
 12 files changed, 268 insertions(+), 7 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/package-note.exp
 create mode 100644 ld/testsuite/ld-elf/package-note.rd

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index c7c0a793b15..65bd1128263 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1918,6 +1918,14 @@ struct output_elf_obj_tdata
     asection *sec;
   } build_id;
 
+  /* FDO_PACKAGING_METADATA note type info.  */
+  struct
+  {
+    bool (*after_write_object_contents) (bfd *);
+    const char *json;
+    asection *sec;
+  } package_metadata;
+
   /* Records the result of `get_program_header_size'.  */
   bfd_size_type program_header_size;
 
diff --git a/bfd/elf.c b/bfd/elf.c
index c493aa5b172..fcc7d14a1d6 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -6779,8 +6779,12 @@ _bfd_elf_write_object_contents (bfd *abfd)
     return false;
 
   /* This is last since write_shdrs_and_ehdr can touch i_shdrp[0].  */
-  if (t->o->build_id.after_write_object_contents != NULL)
-    return (*t->o->build_id.after_write_object_contents) (abfd);
+  if (t->o->build_id.after_write_object_contents != NULL
+       && !(*t->o->build_id.after_write_object_contents) (abfd))
+    return false;
+  if (t->o->package_metadata.after_write_object_contents != NULL
+       && !(*t->o->package_metadata.after_write_object_contents) (abfd))
+    return false;
 
   return true;
 }
diff --git a/ld/Makefile.am b/ld/Makefile.am
index e53bef13bb8..a3cd7891840 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -45,7 +45,7 @@ ELF_CLFAGS=-DELF_LIST_OPTIONS=@elf_list_options@ \
 	   -DELF_PLT_UNWIND_LIST_OPTIONS=@elf_plt_unwind_list_options@
 WARN_CFLAGS = @WARN_CFLAGS@
 NO_WERROR = @NO_WERROR@
-AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS)
+AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS) $(JANSSON_CFLAGS)
 
 # We put the scripts in the directory $(scriptdir)/ldscripts.
 # We can't put the scripts in $(datadir) because the SEARCH_DIR
@@ -964,8 +964,8 @@ ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmai
 	ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \
 	ldbuildid.c
 ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \
-		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP)
-ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB)
+		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP) $(JANSSON_LIBS)
+ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB) $(JANSSON_LIBS)
 
 # Dependency tracking for the generated emulation files.
 EXTRA_ld_new_SOURCES += $(ALL_EMULATION_SOURCES) $(ALL_64_EMULATION_SOURCES)
diff --git a/ld/NEWS b/ld/NEWS
index 514d1d9f207..d2ade345b5d 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -36,6 +36,12 @@
 * Remove (rudimentary) support for the x86-64 sub-architectures Intel L1OM and
   Intel K1OM.
 
+* The ELF linker now supports a new --package-metadata option that allows
+  embedding a JSON payload in accordance to the Package Metadata specification.
+  If libjansson is present at build time, the linker will use it to validate
+  the input. This can be disabled with --disable-jansson.
+  For more details, see: https://systemd.io/ELF_PACKAGE_METADATA/
+
 Changes in 2.38:
 
 * Add -z pack-relative-relocs/-z no pack-relative-relocs to x86 ELF
diff --git a/ld/configure.ac b/ld/configure.ac
index 01121480c6d..a5e7afa89e8 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -289,6 +289,41 @@ fi
 AM_CONDITIONAL(ENABLE_LIBCTF, test "${enable_libctf}" = yes)
 AC_SUBST(enable_libctf)
 
+# If pkg-config and libjansson are available, use them, skip if not.
+# Allow explicitly enabling (error if not found)/disabling (skip even if found).
+AC_ARG_ENABLE([jansson],
+  [AS_HELP_STRING([--enable-jansson],
+    [enable jansson [default=auto]])],
+  [enable_jansson=$enableval],
+  [enable_jansson="auto"])
+
+if test "x$enable_jansson" != "xno"; then
+  PKG_PROG_PKG_CONFIG
+  AS_IF([test -n "$PKG_CONFIG"],
+    [
+      PKG_CHECK_MODULES(JANSSON, [jansson],
+        [
+          AC_DEFINE(HAVE_JANSSON, 1, [The jansson library is to be used])
+          AC_SUBST([JANSSON_CFLAGS])
+          AC_SUBST([JANSSON_LIBS])
+        ],
+        [
+          if test "x$enable_jansson" = "xyes"; then
+            AC_MSG_ERROR([Cannot find jansson library])
+          else
+            AC_MSG_WARN([Cannot find jansson library])
+          fi
+        ])
+    ],
+    [
+      if test "x$enable_jansson" = "xyes"; then
+        AC_MSG_ERROR([Cannot find pkg-config])
+      else
+        AC_MSG_WARN([Cannot find pkg-config])
+      fi
+    ])
+fi
+
 AM_BINUTILS_WARNINGS
 
 AM_LC_MESSAGES
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index c027559908c..c52484f35d3 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -572,6 +572,7 @@ enum elf_options
   OPTION_EXCLUDE_LIBS,
   OPTION_HASH_STYLE,
   OPTION_BUILD_ID,
+  OPTION_PACKAGE_METADATA,
   OPTION_AUDIT,
   OPTION_COMPRESS_DEBUG
 };
@@ -602,6 +603,7 @@ EOF
 fi
 fragment <<EOF
     {"build-id", optional_argument, NULL, OPTION_BUILD_ID},
+    {"package-metadata", optional_argument, NULL, OPTION_PACKAGE_METADATA},
     {"compress-debug-sections", required_argument, NULL, OPTION_COMPRESS_DEBUG},
 EOF
 if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then
@@ -650,6 +652,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
 	ldelf_emit_note_gnu_build_id = xstrdup (optarg);
       break;
 
+    case OPTION_PACKAGE_METADATA:
+      free ((char *) ldelf_emit_note_fdo_package_metadata);
+      ldelf_emit_note_fdo_package_metadata = NULL;
+      if (optarg != NULL && strlen(optarg) > 0)
+	ldelf_emit_note_fdo_package_metadata = xstrdup (optarg);
+      break;
+
     case OPTION_COMPRESS_DEBUG:
       if (strcasecmp (optarg, "none") == 0)
 	link_info.compress_debug = COMPRESS_DEBUG_NONE;
diff --git a/ld/ld.texi b/ld/ld.texi
index a2b162ce5b5..eabbec8faa9 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -2935,6 +2935,18 @@ string identifying the original linked file does not change.
 
 Passing @code{none} for @var{style} disables the setting from any
 @code{--build-id} options earlier on the command line.
+
+@kindex --package-metadata=@var{JSON}
+@item --package-metadata=@var{JSON}
+Request the creation of a @code{.note.package} ELF note section.  The
+contents of the note are in JSON format, as per the package metadata
+specification.  For more information see:
+https://systemd.io/ELF_PACKAGE_METADATA/
+If the JSON argument is missing/empty then this will disable the
+creation of the metadata note, if one had been enabled by an earlier
+occurrence of the --package-metdata option.
+If the linker has been built with libjansson, then the JSON string
+will be validated.
 @end table
 
 @c man end
diff --git a/ld/ldelf.c b/ld/ldelf.c
index 4094640b3f7..e5d1b32db06 100644
--- a/ld/ldelf.c
+++ b/ld/ldelf.c
@@ -39,6 +39,9 @@
 #include <glob.h>
 #endif
 #include "ldelf.h"
+#ifdef HAVE_JANSSON
+#include <jansson.h>
+#endif
 
 struct dt_needed
 {
@@ -49,6 +52,9 @@ struct dt_needed
 /* Style of .note.gnu.build-id section.  */
 const char *ldelf_emit_note_gnu_build_id;
 
+/* Content of .note.package section.  */
+const char *ldelf_emit_note_fdo_package_metadata;
+
 /* These variables are required to pass information back and forth
    between after_open and check_needed and stat_needed and vercheck.  */
 
@@ -1249,7 +1255,8 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
 	}
     }
 
-  if (ldelf_emit_note_gnu_build_id != NULL)
+  if (ldelf_emit_note_gnu_build_id != NULL
+      || ldelf_emit_note_fdo_package_metadata != NULL)
     {
       /* Find an ELF input.  */
       for (abfd = link_info.input_bfds;
@@ -1262,11 +1269,18 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
       /* PR 10555: If there are no ELF input files do not try to
 	 create a .note.gnu-build-id section.  */
       if (abfd == NULL
-	  || !ldelf_setup_build_id (abfd))
+	  || (ldelf_emit_note_gnu_build_id != NULL && !ldelf_setup_build_id (abfd)))
 	{
 	  free ((char *) ldelf_emit_note_gnu_build_id);
 	  ldelf_emit_note_gnu_build_id = NULL;
 	}
+
+      if (abfd == NULL
+	  || (ldelf_emit_note_fdo_package_metadata != NULL && !ldelf_setup_package_metadata (abfd)))
+	{
+	  free ((char *) ldelf_emit_note_fdo_package_metadata);
+	  ldelf_emit_note_fdo_package_metadata = NULL;
+	}
     }
 
   get_elf_backend_data (link_info.output_bfd)->setup_gnu_properties (&link_info);
@@ -1501,6 +1515,124 @@ ldelf_setup_build_id (bfd *ibfd)
   return false;
 }
 
+static bool
+write_package_metadata (bfd *abfd)
+{
+  struct elf_obj_tdata *t = elf_tdata (abfd);
+  const char *json;
+  asection *asec;
+  Elf_Internal_Shdr *i_shdr;
+  unsigned char *contents, *json_bits;
+  bfd_size_type size;
+  file_ptr position;
+  Elf_External_Note *e_note;
+
+  json = t->o->package_metadata.json;
+  asec = t->o->package_metadata.sec;
+  if (bfd_is_abs_section (asec->output_section))
+    {
+      einfo (_("%P: warning: .note.package section discarded,"
+	       " --package-metadata ignored\n"));
+      return true;
+    }
+  i_shdr = &elf_section_data (asec->output_section)->this_hdr;
+
+  if (i_shdr->contents == NULL)
+    {
+      if (asec->contents == NULL)
+	asec->contents = (unsigned char *) xmalloc (asec->size);
+      contents = asec->contents;
+    }
+  else
+    contents = i_shdr->contents + asec->output_offset;
+
+  e_note = (Elf_External_Note *) contents;
+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
+  size = (size + 3) & -(bfd_size_type) 4;
+  json_bits = contents + size;
+  size = asec->size - size;
+
+  /* Clear the package metadata field.  */
+  memset (json_bits, 0, size);
+
+  bfd_h_put_32 (abfd, sizeof "FDO", &e_note->namesz);
+  bfd_h_put_32 (abfd, size, &e_note->descsz);
+  bfd_h_put_32 (abfd, FDO_PACKAGING_METADATA, &e_note->type);
+  memcpy (e_note->name, "FDO", sizeof "FDO");
+  memcpy (json_bits, json, strlen(json));
+
+  position = i_shdr->sh_offset + asec->output_offset;
+  size = asec->size;
+  return (bfd_seek (abfd, position, SEEK_SET) == 0
+	  && bfd_bwrite (contents, size, abfd) == size);
+}
+
+/* Make .note.package section.
+   https://systemd.io/ELF_PACKAGE_METADATA/  */
+
+bool
+ldelf_setup_package_metadata (bfd *ibfd)
+{
+  asection *s;
+  bfd_size_type size;
+  size_t json_length;
+  flagword flags;
+
+  /* If the option wasn't specified, silently return. */
+  if (!ldelf_emit_note_fdo_package_metadata)
+    return false;
+
+  /* The option was specified, but it's too short/long, log and return. */
+  json_length = strlen (ldelf_emit_note_fdo_package_metadata);
+  if (json_length == 0)
+    {
+      einfo (_("%P: warning: --package-metadata is empty, ignoring\n"));
+      return false;
+    }
+  if (json_length > SIZE_MAX)
+    {
+      einfo (_("%P: warning: --package-metadata is too long, ignoring\n"));
+      return false;
+    }
+
+#ifdef HAVE_JANSSON
+  json_error_t json_error;
+  json_t *json = json_loads (ldelf_emit_note_fdo_package_metadata, 0, &json_error);
+  if (!json)
+    {
+      einfo (_("%P: warning: --package-metadata=%s does not contain valid "
+	       "JSON, ignoring: %s\n"), ldelf_emit_note_fdo_package_metadata,
+         json_error.text);
+      return false;
+    }
+  else
+    json_decref (json);
+#endif
+
+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
+  size += json_length + 1;
+  size = (size + 3) & -(bfd_size_type) 4;
+
+  flags = (SEC_ALLOC | SEC_LOAD | SEC_IN_MEMORY
+	   | SEC_LINKER_CREATED | SEC_READONLY | SEC_DATA);
+  s = bfd_make_section_anyway_with_flags (ibfd, ".note.package",
+					  flags);
+  if (s != NULL && bfd_set_section_alignment (s, 2))
+    {
+      struct elf_obj_tdata *t = elf_tdata (link_info.output_bfd);
+      t->o->package_metadata.after_write_object_contents = &write_package_metadata;
+      t->o->package_metadata.json = ldelf_emit_note_fdo_package_metadata;
+      t->o->package_metadata.sec = s;
+      elf_section_type (s) = SHT_NOTE;
+      s->size = size;
+      return true;
+    }
+
+  einfo (_("%P: warning: cannot create .note.package section,"
+	   " --package-metadata ignored\n"));
+  return false;
+}
+
 /* Look through an expression for an assignment statement.  */
 
 static void
diff --git a/ld/ldelf.h b/ld/ldelf.h
index efa8b45851c..a3ded3dd04c 100644
--- a/ld/ldelf.h
+++ b/ld/ldelf.h
@@ -19,6 +19,7 @@
    MA 02110-1301, USA.  */
 
 extern const char *ldelf_emit_note_gnu_build_id;
+extern const char *ldelf_emit_note_fdo_package_metadata;
 
 extern void ldelf_after_parse (void);
 extern bool ldelf_load_symbols (lang_input_statement_type *);
@@ -26,6 +27,7 @@ extern void ldelf_before_plugin_all_symbols_read (int, int, int, int,
 						  int, const char *);
 extern void ldelf_after_open (int, int, int, int, int, const char *);
 extern bool ldelf_setup_build_id (bfd *);
+extern bool ldelf_setup_package_metadata (bfd *);
 extern void ldelf_append_to_separated_string (char **, char *);
 extern void ldelf_before_allocation (char *, char *, const char *);
 extern bool ldelf_open_dynamic_archive
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 82c459adb51..9225f71b3ce 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2144,6 +2144,8 @@ elf_static_list_options (FILE *file)
   fprintf (file, _("\
   --build-id[=STYLE]          Generate build ID note\n"));
   fprintf (file, _("\
+  --package-metadata[=JSON]   Generate package metadata note\n"));
+  fprintf (file, _("\
   --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi]\n\
                               Compress DWARF debug sections using zlib\n"));
 #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
diff --git a/ld/testsuite/ld-elf/package-note.exp b/ld/testsuite/ld-elf/package-note.exp
new file mode 100644
index 00000000000..c4239098691
--- /dev/null
+++ b/ld/testsuite/ld-elf/package-note.exp
@@ -0,0 +1,45 @@
+# Expect script for --package-note tests.
+#   Copyright (C) 2022 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# 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 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+# Exclude non-ELF targets.
+
+if ![is_elf_format] {
+    return
+}
+
+if { !([istarget *-*-linux*]
+       || [istarget arm*-*-uclinuxfdpiceabi]
+       || [istarget *-*-nacl*]
+       || [istarget *-*-gnu*]) } then {
+    return
+}
+
+run_ld_link_tests [list \
+    [list \
+	"package-note.o" \
+	"--package-metadata='{\"foo\":\"bar\"}'" \
+	"" \
+	"" \
+	{start.s} \
+	{{readelf {--notes} package-note.rd}} \
+	"package-note.o" \
+    ] \
+]
diff --git a/ld/testsuite/ld-elf/package-note.rd b/ld/testsuite/ld-elf/package-note.rd
new file mode 100644
index 00000000000..c65dd21cdac
--- /dev/null
+++ b/ld/testsuite/ld-elf/package-note.rd
@@ -0,0 +1,6 @@
+#...
+Displaying notes found in: \.note\.package
+  Owner                Data size 	Description
+  FDO                  0x00000010	FDO_PACKAGING_METADATA
+    Packaging Metadata: {"foo":"bar"}
+#pass
-- 
2.35.1

Re: [PATCH v2] ld: add --package-metadata

[Alan Modra]

On Tue, May 24, 2022 at 10:15:07PM +0100, luca.boccassi--- via Binutils wrote:
> v2: fix style issues
>     fix ASAN issue: note storage is rounded up, but that size
>     was used to read the json input, which is fixed
>     add entry to NEWS
>     add note about json validation to docs

Looks OK to me

> +  if (json_length > SIZE_MAX)
> +    {
> +      einfo (_("%P: warning: --package-metadata is too long, ignoring\n"));
> +      return false;
> +    }

except for the above dead code.  OK with that removed.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH v2] ld: add --package-metadata

[Alan Modra]

On Wed, May 25, 2022 at 02:00:08PM +0930, Alan Modra wrote:
> On Tue, May 24, 2022 at 10:15:07PM +0100, luca.boccassi--- via Binutils wrote:
> > v2: fix style issues
> >     fix ASAN issue: note storage is rounded up, but that size
> >     was used to read the json input, which is fixed
> >     add entry to NEWS
> >     add note about json validation to docs
> 
> Looks OK to me
> 
> > +  if (json_length > SIZE_MAX)
> > +    {
> > +      einfo (_("%P: warning: --package-metadata is too long, ignoring\n"));
> > +      return false;
> > +    }
> 
> except for the above dead code.  OK with that removed.

And how having applied your patch locally and tested, please do look
into fixing
FAIL: bootstrap
FAIL: bootstrap with strip
FAIL: bootstrap with -Wl,--traditional-format
FAIL: bootstrap with -Wl,--no-keep-memory
FAIL: bootstrap with -Wl,--relax
FAIL: bootstrap with -Wl,--max-cache-size=-1
seen with #define HAVE_JANSSON 1

The errors are of course all due to undefined references to libjansson
symbols.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] ld: add --package-metadata

[Fangrui Song]

On 2022-05-17, Luca Boccassi via Binutils wrote:
>On Tue, 17 May 2022 at 07:03, Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Fangrui Song:
>>
>> > Both the "FDO" and the json dependency make me concerned -
>> > if a linker script approach works quite well, why bother with a new
>> > linker option with a very specific application?
>>
>> The linker script does not work all that well because it requires that
>> the script is materialized to disk somewhere, and that path needs to
>> show up in the build flags.  If the build flags are stored beyond the
>> build for future use, this makes them invalid because the path will
>> typically be gone by the time the stored flags are used.
>>
>> Thanks,
>> Florian

Do you mean that a user may need to re-link the executable/shared
object? The note content must be preserved somewhere, I don't see how

as ... -o meta.o
ld.bfd ... meta.o

is more inconvenient than

ld.bfd --package-meta='content' ...

The "FDO" owner also makes me concerned. It seems a bit arbitrary.

https://src.fedoraproject.org/rpms/package-notes/pull-request/2#comment-98855 (tstellar)
says
"Given, that I'm the maintainer of lld in Fedora, if I volunteer to deal
with issues that stem from this assembler based implementation would you
be more likely to accept this patch?"

It would be nice to have a list of packages and fix them (perhaps issues like not honor LDFLAGS).

I left a comment on
https://github.com/rui314/mold/issues/505#issuecomment-1136277017 as
well.

>Indeed, it was working fine as an initial step to get the ball rolling and
>the spec adopted, but having to generate and reference an
>intermediate, temporary asset is not great and has drawbacks. Now
>that the spec is live and proven in production in Fedora 36 it seemed
>to me like a good time to introduce a first-class option, to make further
>usage and adoption as trivial as it can be.
>
>Kind regards,
>Luca Boccassi

Re: [PATCH] ld: add --package-metadata

[Florian Weimer]

* Fangrui Song:

> On 2022-05-17, Luca Boccassi via Binutils wrote:
>>On Tue, 17 May 2022 at 07:03, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * Fangrui Song:
>>>
>>> > Both the "FDO" and the json dependency make me concerned -
>>> > if a linker script approach works quite well, why bother with a new
>>> > linker option with a very specific application?
>>>
>>> The linker script does not work all that well because it requires that
>>> the script is materialized to disk somewhere, and that path needs to
>>> show up in the build flags.  If the build flags are stored beyond the
>>> build for future use, this makes them invalid because the path will
>>> typically be gone by the time the stored flags are used.
>>>
>>> Thanks,
>>> Florian
>
> Do you mean that a user may need to re-link the executable/shared
> object? The note content must be preserved somewhere, I don't see how
>
> as ... -o meta.o
> ld.bfd ... meta.o
>
> is more inconvenient than
>
> ld.bfd --package-meta='content' ...

If the object file is put into /build/builddir/BUILD/foo-12.3/meta.o,
the full path ends up in LDFLAGS.  (The validity of a relative path
would depend on the package-specific build system.)  But if we do that,
and the package build system captures LDFLAGS into some generated
artifact that controls link editor invocation, it may try to link with
/build/builddir/BUILD/foo-12.3/meta.o.  But that file will no longer
exist at the time.

> The "FDO" owner also makes me concerned. It seems a bit arbitrary.

It's the usual abbreviation for freedesktop.org, where the specification
lives.

> https://src.fedoraproject.org/rpms/package-notes/pull-request/2#comment-98855 (tstellar)
> says
> "Given, that I'm the maintainer of lld in Fedora, if I volunteer to deal
> with issues that stem from this assembler based implementation would you
> be more likely to accept this patch?"
>
> It would be nice to have a list of packages and fix them (perhaps
> issues like not honor LDFLAGS).

I think this is unrelated to honoring LDFLAGS or not.

Thanks,
Florian

Re: [PATCH] ld: add --package-metadata

[Florian Weimer]

* Luca Boccassi:

> So with that experience in the bag, the most obvious next step is to
> have a first-class option, that allows a self-contained flag to be
> passed instead. After all the idea is similar to the build-id, and
> there we have a first-class option too, and it works well.

Maybe it's better to just pre-allocate space for the program header note
(and corresponding data) and patch the actual contents in later, maybe
as part of the debuginfo post-processing.

This would also side-step any shell encoding issues of the JSON object.

A really nice approach would require changes to the way we generate
coredumps: teach the dumper to copy non-allocated sections from a file
region.  Then we wouldn't have to pre-allocate section contents at all.
I think the core dumper would have to look at section headers for this,
not program headers.  We could add a program header that describes the
file region to be copied, but it would get out of sync with the file
contents if ELF editing tools don't know that it has to be updated if
non-allocated sections are changed.

Thanks,
Florian

[PATCH v3] ld: add --package-metadata

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

Generate a .note.package FDO package metadata ELF note, following
the spec: https://systemd.io/ELF_PACKAGE_METADATA/

If the jansson library is available at build time (and it is explicitly
enabled), link ld to it, and use it to validate that the input is
correct JSON, to avoid writing garbage to the file. The
configure option --enable-jansson has to be used to explicitly enable
it (error out when not found). This allows bootstrappers (or others who
are not interested) to seamlessly skip it without issues.
---
v2: fix style issues
    fix ASAN issue: note storage is rounded up, but that size
    was used to read the json input, which is fixed
    add entry to NEWS
    add note about json validation to docs
v3: switch libjansson support to disabled-by-default even if
      the library is available in the build environment
    update new test to account for old/new readelf, which
      might or might not pretty-print the FDO note
    update bootstrap test to link with jansson if the object
      files being linked were built with it
    remove dead code

 bfd/elf-bfd.h                           |   8 ++
 bfd/elf.c                               |   8 +-
 ld/Makefile.am                          |   6 +-
 ld/NEWS                                 |   6 ++
 ld/configure.ac                         |  26 +++++
 ld/emultempl/elf.em                     |   9 ++
 ld/ld.texi                              |  12 +++
 ld/ldelf.c                              | 131 +++++++++++++++++++++++-
 ld/ldelf.h                              |   2 +
 ld/lexsup.c                             |   2 +
 ld/testsuite/ld-bootstrap/bootstrap.exp |   6 ++
 ld/testsuite/ld-elf/package-note.exp    |  45 ++++++++
 ld/testsuite/ld-elf/package-note.rd     |   6 ++
 13 files changed, 260 insertions(+), 7 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/package-note.exp
 create mode 100644 ld/testsuite/ld-elf/package-note.rd

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index c7c0a793b15..65bd1128263 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1918,6 +1918,14 @@ struct output_elf_obj_tdata
     asection *sec;
   } build_id;
 
+  /* FDO_PACKAGING_METADATA note type info.  */
+  struct
+  {
+    bool (*after_write_object_contents) (bfd *);
+    const char *json;
+    asection *sec;
+  } package_metadata;
+
   /* Records the result of `get_program_header_size'.  */
   bfd_size_type program_header_size;
 
diff --git a/bfd/elf.c b/bfd/elf.c
index c493aa5b172..fcc7d14a1d6 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -6779,8 +6779,12 @@ _bfd_elf_write_object_contents (bfd *abfd)
     return false;
 
   /* This is last since write_shdrs_and_ehdr can touch i_shdrp[0].  */
-  if (t->o->build_id.after_write_object_contents != NULL)
-    return (*t->o->build_id.after_write_object_contents) (abfd);
+  if (t->o->build_id.after_write_object_contents != NULL
+       && !(*t->o->build_id.after_write_object_contents) (abfd))
+    return false;
+  if (t->o->package_metadata.after_write_object_contents != NULL
+       && !(*t->o->package_metadata.after_write_object_contents) (abfd))
+    return false;
 
   return true;
 }
diff --git a/ld/Makefile.am b/ld/Makefile.am
index e53bef13bb8..a3cd7891840 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -45,7 +45,7 @@ ELF_CLFAGS=-DELF_LIST_OPTIONS=@elf_list_options@ \
 	   -DELF_PLT_UNWIND_LIST_OPTIONS=@elf_plt_unwind_list_options@
 WARN_CFLAGS = @WARN_CFLAGS@
 NO_WERROR = @NO_WERROR@
-AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS)
+AM_CFLAGS = $(WARN_CFLAGS) $(ELF_CLFAGS) $(JANSSON_CFLAGS)
 
 # We put the scripts in the directory $(scriptdir)/ldscripts.
 # We can't put the scripts in $(datadir) because the SEARCH_DIR
@@ -964,8 +964,8 @@ ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmai
 	ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \
 	ldbuildid.c
 ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \
-		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP)
-ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB)
+		      $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL_DEP) $(JANSSON_LIBS)
+ld_new_LDADD = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) $(BFDLIB) $(LIBCTF) $(LIBIBERTY) $(LIBINTL) $(ZLIB) $(JANSSON_LIBS)
 
 # Dependency tracking for the generated emulation files.
 EXTRA_ld_new_SOURCES += $(ALL_EMULATION_SOURCES) $(ALL_64_EMULATION_SOURCES)
diff --git a/ld/NEWS b/ld/NEWS
index 514d1d9f207..d580825c0f7 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -36,6 +36,12 @@
 * Remove (rudimentary) support for the x86-64 sub-architectures Intel L1OM and
   Intel K1OM.
 
+* The ELF linker now supports a new --package-metadata option that allows
+  embedding a JSON payload in accordance to the Package Metadata specification.
+  If support for libjansson is enabled at build time, the linker will use it to
+  validate the input. This can be enabled with --enable-jansson.
+  For more details, see: https://systemd.io/ELF_PACKAGE_METADATA/
+
 Changes in 2.38:
 
 * Add -z pack-relative-relocs/-z no pack-relative-relocs to x86 ELF
diff --git a/ld/configure.ac b/ld/configure.ac
index 01121480c6d..6acb029a354 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -289,6 +289,32 @@ fi
 AM_CONDITIONAL(ENABLE_LIBCTF, test "${enable_libctf}" = yes)
 AC_SUBST(enable_libctf)
 
+# Used to validate --package-metadata= input. Disabled by default.
+AC_ARG_ENABLE([jansson],
+  [AS_HELP_STRING([--enable-jansson],
+    [enable jansson [default=no]])],
+  [enable_jansson=$enableval],
+  [enable_jansson="no"])
+
+if test "x$enable_jansson" != "xno"; then
+  PKG_PROG_PKG_CONFIG
+  AS_IF([test -n "$PKG_CONFIG"],
+    [
+      PKG_CHECK_MODULES(JANSSON, [jansson],
+        [
+          AC_DEFINE(HAVE_JANSSON, 1, [The jansson library is to be used])
+          AC_SUBST([JANSSON_CFLAGS])
+          AC_SUBST([JANSSON_LIBS])
+        ],
+        [
+          AC_MSG_ERROR([Cannot find jansson library])
+        ])
+    ],
+    [
+      AC_MSG_ERROR([Cannot find pkg-config])
+    ])
+fi
+
 AM_BINUTILS_WARNINGS
 
 AM_LC_MESSAGES
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index c027559908c..c52484f35d3 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -572,6 +572,7 @@ enum elf_options
   OPTION_EXCLUDE_LIBS,
   OPTION_HASH_STYLE,
   OPTION_BUILD_ID,
+  OPTION_PACKAGE_METADATA,
   OPTION_AUDIT,
   OPTION_COMPRESS_DEBUG
 };
@@ -602,6 +603,7 @@ EOF
 fi
 fragment <<EOF
     {"build-id", optional_argument, NULL, OPTION_BUILD_ID},
+    {"package-metadata", optional_argument, NULL, OPTION_PACKAGE_METADATA},
     {"compress-debug-sections", required_argument, NULL, OPTION_COMPRESS_DEBUG},
 EOF
 if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then
@@ -650,6 +652,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
 	ldelf_emit_note_gnu_build_id = xstrdup (optarg);
       break;
 
+    case OPTION_PACKAGE_METADATA:
+      free ((char *) ldelf_emit_note_fdo_package_metadata);
+      ldelf_emit_note_fdo_package_metadata = NULL;
+      if (optarg != NULL && strlen(optarg) > 0)
+	ldelf_emit_note_fdo_package_metadata = xstrdup (optarg);
+      break;
+
     case OPTION_COMPRESS_DEBUG:
       if (strcasecmp (optarg, "none") == 0)
 	link_info.compress_debug = COMPRESS_DEBUG_NONE;
diff --git a/ld/ld.texi b/ld/ld.texi
index a2b162ce5b5..eabbec8faa9 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -2935,6 +2935,18 @@ string identifying the original linked file does not change.
 
 Passing @code{none} for @var{style} disables the setting from any
 @code{--build-id} options earlier on the command line.
+
+@kindex --package-metadata=@var{JSON}
+@item --package-metadata=@var{JSON}
+Request the creation of a @code{.note.package} ELF note section.  The
+contents of the note are in JSON format, as per the package metadata
+specification.  For more information see:
+https://systemd.io/ELF_PACKAGE_METADATA/
+If the JSON argument is missing/empty then this will disable the
+creation of the metadata note, if one had been enabled by an earlier
+occurrence of the --package-metdata option.
+If the linker has been built with libjansson, then the JSON string
+will be validated.
 @end table
 
 @c man end
diff --git a/ld/ldelf.c b/ld/ldelf.c
index 4094640b3f7..4b7c064ff8d 100644
--- a/ld/ldelf.c
+++ b/ld/ldelf.c
@@ -39,6 +39,9 @@
 #include <glob.h>
 #endif
 #include "ldelf.h"
+#ifdef HAVE_JANSSON
+#include <jansson.h>
+#endif
 
 struct dt_needed
 {
@@ -49,6 +52,9 @@ struct dt_needed
 /* Style of .note.gnu.build-id section.  */
 const char *ldelf_emit_note_gnu_build_id;
 
+/* Content of .note.package section.  */
+const char *ldelf_emit_note_fdo_package_metadata;
+
 /* These variables are required to pass information back and forth
    between after_open and check_needed and stat_needed and vercheck.  */
 
@@ -1249,7 +1255,8 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
 	}
     }
 
-  if (ldelf_emit_note_gnu_build_id != NULL)
+  if (ldelf_emit_note_gnu_build_id != NULL
+      || ldelf_emit_note_fdo_package_metadata != NULL)
     {
       /* Find an ELF input.  */
       for (abfd = link_info.input_bfds;
@@ -1262,11 +1269,18 @@ ldelf_after_open (int use_libpath, int native, int is_linux, int is_freebsd,
       /* PR 10555: If there are no ELF input files do not try to
 	 create a .note.gnu-build-id section.  */
       if (abfd == NULL
-	  || !ldelf_setup_build_id (abfd))
+	  || (ldelf_emit_note_gnu_build_id != NULL && !ldelf_setup_build_id (abfd)))
 	{
 	  free ((char *) ldelf_emit_note_gnu_build_id);
 	  ldelf_emit_note_gnu_build_id = NULL;
 	}
+
+      if (abfd == NULL
+	  || (ldelf_emit_note_fdo_package_metadata != NULL && !ldelf_setup_package_metadata (abfd)))
+	{
+	  free ((char *) ldelf_emit_note_fdo_package_metadata);
+	  ldelf_emit_note_fdo_package_metadata = NULL;
+	}
     }
 
   get_elf_backend_data (link_info.output_bfd)->setup_gnu_properties (&link_info);
@@ -1501,6 +1515,119 @@ ldelf_setup_build_id (bfd *ibfd)
   return false;
 }
 
+static bool
+write_package_metadata (bfd *abfd)
+{
+  struct elf_obj_tdata *t = elf_tdata (abfd);
+  const char *json;
+  asection *asec;
+  Elf_Internal_Shdr *i_shdr;
+  unsigned char *contents, *json_bits;
+  bfd_size_type size;
+  file_ptr position;
+  Elf_External_Note *e_note;
+
+  json = t->o->package_metadata.json;
+  asec = t->o->package_metadata.sec;
+  if (bfd_is_abs_section (asec->output_section))
+    {
+      einfo (_("%P: warning: .note.package section discarded,"
+	       " --package-metadata ignored\n"));
+      return true;
+    }
+  i_shdr = &elf_section_data (asec->output_section)->this_hdr;
+
+  if (i_shdr->contents == NULL)
+    {
+      if (asec->contents == NULL)
+	asec->contents = (unsigned char *) xmalloc (asec->size);
+      contents = asec->contents;
+    }
+  else
+    contents = i_shdr->contents + asec->output_offset;
+
+  e_note = (Elf_External_Note *) contents;
+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
+  size = (size + 3) & -(bfd_size_type) 4;
+  json_bits = contents + size;
+  size = asec->size - size;
+
+  /* Clear the package metadata field.  */
+  memset (json_bits, 0, size);
+
+  bfd_h_put_32 (abfd, sizeof "FDO", &e_note->namesz);
+  bfd_h_put_32 (abfd, size, &e_note->descsz);
+  bfd_h_put_32 (abfd, FDO_PACKAGING_METADATA, &e_note->type);
+  memcpy (e_note->name, "FDO", sizeof "FDO");
+  memcpy (json_bits, json, strlen(json));
+
+  position = i_shdr->sh_offset + asec->output_offset;
+  size = asec->size;
+  return (bfd_seek (abfd, position, SEEK_SET) == 0
+	  && bfd_bwrite (contents, size, abfd) == size);
+}
+
+/* Make .note.package section.
+   https://systemd.io/ELF_PACKAGE_METADATA/  */
+
+bool
+ldelf_setup_package_metadata (bfd *ibfd)
+{
+  asection *s;
+  bfd_size_type size;
+  size_t json_length;
+  flagword flags;
+
+  /* If the option wasn't specified, silently return. */
+  if (!ldelf_emit_note_fdo_package_metadata)
+    return false;
+
+  /* The option was specified, but it's empty, log and return. */
+  json_length = strlen (ldelf_emit_note_fdo_package_metadata);
+  if (json_length == 0)
+    {
+      einfo (_("%P: warning: --package-metadata is empty, ignoring\n"));
+      return false;
+    }
+
+#ifdef HAVE_JANSSON
+  json_error_t json_error;
+  json_t *json = json_loads (ldelf_emit_note_fdo_package_metadata, 0, &json_error);
+  if (!json)
+    {
+      einfo (_("%P: warning: --package-metadata=%s does not contain valid "
+	       "JSON, ignoring: %s\n"), ldelf_emit_note_fdo_package_metadata,
+         json_error.text);
+      return false;
+    }
+  else
+    json_decref (json);
+#endif
+
+  size = offsetof (Elf_External_Note, name[sizeof "FDO"]);
+  size += json_length + 1;
+  size = (size + 3) & -(bfd_size_type) 4;
+
+  flags = (SEC_ALLOC | SEC_LOAD | SEC_IN_MEMORY
+	   | SEC_LINKER_CREATED | SEC_READONLY | SEC_DATA);
+  s = bfd_make_section_anyway_with_flags (ibfd, ".note.package",
+					  flags);
+  if (s != NULL && bfd_set_section_alignment (s, 2))
+    {
+      struct elf_obj_tdata *t = elf_tdata (link_info.output_bfd);
+      t->o->package_metadata.after_write_object_contents = &write_package_metadata;
+      t->o->package_metadata.json = ldelf_emit_note_fdo_package_metadata;
+      t->o->package_metadata.sec = s;
+      elf_section_type (s) = SHT_NOTE;
+      s->size = size;
+      return true;
+    }
+
+  einfo (_("%P: warning: cannot create .note.package section,"
+	   " --package-metadata ignored\n"));
+  return false;
+}
+
 /* Look through an expression for an assignment statement.  */
 
 static void
diff --git a/ld/ldelf.h b/ld/ldelf.h
index efa8b45851c..a3ded3dd04c 100644
--- a/ld/ldelf.h
+++ b/ld/ldelf.h
@@ -19,6 +19,7 @@
    MA 02110-1301, USA.  */
 
 extern const char *ldelf_emit_note_gnu_build_id;
+extern const char *ldelf_emit_note_fdo_package_metadata;
 
 extern void ldelf_after_parse (void);
 extern bool ldelf_load_symbols (lang_input_statement_type *);
@@ -26,6 +27,7 @@ extern void ldelf_before_plugin_all_symbols_read (int, int, int, int,
 						  int, const char *);
 extern void ldelf_after_open (int, int, int, int, int, const char *);
 extern bool ldelf_setup_build_id (bfd *);
+extern bool ldelf_setup_package_metadata (bfd *);
 extern void ldelf_append_to_separated_string (char **, char *);
 extern void ldelf_before_allocation (char *, char *, const char *);
 extern bool ldelf_open_dynamic_archive
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 82c459adb51..9225f71b3ce 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2144,6 +2144,8 @@ elf_static_list_options (FILE *file)
   fprintf (file, _("\
   --build-id[=STYLE]          Generate build ID note\n"));
   fprintf (file, _("\
+  --package-metadata[=JSON]   Generate package metadata note\n"));
+  fprintf (file, _("\
   --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi]\n\
                               Compress DWARF debug sections using zlib\n"));
 #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
diff --git a/ld/testsuite/ld-bootstrap/bootstrap.exp b/ld/testsuite/ld-bootstrap/bootstrap.exp
index bc83db6556f..7fcf1fabd47 100644
--- a/ld/testsuite/ld-bootstrap/bootstrap.exp
+++ b/ld/testsuite/ld-bootstrap/bootstrap.exp
@@ -155,6 +155,12 @@ foreach flags $test_flags {
 	set extralibs "$extralibs -lz"
     }
 
+    # Check if the system's jansson library is used. If so, the object files will
+    # be using symbols from it, so link to it.
+    if { [lindex [remote_exec host fgrep "-q \"HAVE_JANSSON 1\" $srcdir/../config.h" ] 0] == 0 } then {
+	set extralibs "$extralibs -ljansson"
+    }
+
     # Plugin support requires linking with libdl.
     if { $plugins == "yes" } {
 	if { ![istarget "*-*-freebsd*"]} {
diff --git a/ld/testsuite/ld-elf/package-note.exp b/ld/testsuite/ld-elf/package-note.exp
new file mode 100644
index 00000000000..c4239098691
--- /dev/null
+++ b/ld/testsuite/ld-elf/package-note.exp
@@ -0,0 +1,45 @@
+# Expect script for --package-note tests.
+#   Copyright (C) 2022 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# 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 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+# Exclude non-ELF targets.
+
+if ![is_elf_format] {
+    return
+}
+
+if { !([istarget *-*-linux*]
+       || [istarget arm*-*-uclinuxfdpiceabi]
+       || [istarget *-*-nacl*]
+       || [istarget *-*-gnu*]) } then {
+    return
+}
+
+run_ld_link_tests [list \
+    [list \
+	"package-note.o" \
+	"--package-metadata='{\"foo\":\"bar\"}'" \
+	"" \
+	"" \
+	{start.s} \
+	{{readelf {--notes} package-note.rd}} \
+	"package-note.o" \
+    ] \
+]
diff --git a/ld/testsuite/ld-elf/package-note.rd b/ld/testsuite/ld-elf/package-note.rd
new file mode 100644
index 00000000000..77ae4733ece
--- /dev/null
+++ b/ld/testsuite/ld-elf/package-note.rd
@@ -0,0 +1,6 @@
+#...
+Displaying notes found in: \.note\.package
+\s+Owner\s+Data\s+size\s+Description
+\s+FDO\s+0x00000010\s+(Unknown note type:\s+\(0xcafe1a7e\)|FDO_PACKAGING_METADATA)
+\s+(description data:\s+.*|Packaging Metadata:\s+{"foo":"bar"})
+#pass
-- 
2.34.1

Re: [PATCH v2] ld: add --package-metadata

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

On Wed, 2022-05-25 at 15:32 +0930, Alan Modra wrote:
> On Wed, May 25, 2022 at 02:00:08PM +0930, Alan Modra wrote:
> > On Tue, May 24, 2022 at 10:15:07PM +0100, luca.boccassi--- via Binutils wrote:
> > > v2: fix style issues
> > >     fix ASAN issue: note storage is rounded up, but that size
> > >     was used to read the json input, which is fixed
> > >     add entry to NEWS
> > >     add note about json validation to docs
> > 
> > Looks OK to me
> > 
> > > +  if (json_length > SIZE_MAX)
> > > +    {
> > > +      einfo (_("%P: warning: --package-metadata is too long, ignoring\n"));
> > > +      return false;
> > > +    }
> > 
> > except for the above dead code.  OK with that removed.
> 
> And how having applied your patch locally and tested, please do look
> into fixing
> FAIL: bootstrap
> FAIL: bootstrap with strip
> FAIL: bootstrap with -Wl,--traditional-format
> FAIL: bootstrap with -Wl,--no-keep-memory
> FAIL: bootstrap with -Wl,--relax
> FAIL: bootstrap with -Wl,--max-cache-size=-1
> seen with #define HAVE_JANSSON 1
> 
> The errors are of course all due to undefined references to libjansson
> symbols.

Thanks for the review, I've fixed both issues in v3.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] ld: add --package-metadata

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2632 bytes --]

On Wed, 2022-05-25 at 10:45 +0200, Florian Weimer wrote:
> * Luca Boccassi:
> 
> > So with that experience in the bag, the most obvious next step is to
> > have a first-class option, that allows a self-contained flag to be
> > passed instead. After all the idea is similar to the build-id, and
> > there we have a first-class option too, and it works well.
> 
> Maybe it's better to just pre-allocate space for the program header note
> (and corresponding data) and patch the actual contents in later, maybe
> as part of the debuginfo post-processing.
> 
> This would also side-step any shell encoding issues of the JSON object.

I'm not sure - that still requires to do a lot of the work here, while
also leaving each and every distro builder to implement a whole load of
integration on their own, risking divergence to appear at some point,
while allowing for many more things to go wrong at multiple stages. The
advantage of having a common, standard and shared interface is that
every distro that opts-in will produce the same results, so that
tooling doesn't need to take these differences into account. And it
either works or not, in a single step.
The advantages would be very minimal - there is already knowledge about
specific fields, this was pretty much modelled on the existing --build-
id support, and it would still require implementation support here.

Given there seems to be some concerns with the jansson integration,
I've flipped it in v3 to be default-disabled, regardless of the status
of the build env. It now is explicit opt-in. I hope that makes the
patch more palatable?

> A really nice approach would require changes to the way we generate
> coredumps: teach the dumper to copy non-allocated sections from a file
> region.  Then we wouldn't have to pre-allocate section contents at all.
> I think the core dumper would have to look at section headers for this,
> not program headers.  We could add a program header that describes the
> file region to be copied, but it would get out of sync with the file
> contents if ELF editing tools don't know that it has to be updated if
> non-allocated sections are changed.

Requiring such sweeping changes in the kernel (it would be a default
behaviour change, whether one opts in to this or not), which means they
need to be propagated everywhere, would take years at best, even if it
was possible, acceptable and it were to actually happen. Also, the
consumer implementation would no longer be compatible with what we have
published now in production in CBL-Mariner and Fedora.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] ld: add --package-metadata

[Alan Modra]

On Wed, May 25, 2022 at 02:41:47PM +0100, luca.boccassi@gmail.com wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> Generate a .note.package FDO package metadata ELF note, following
> the spec: https://systemd.io/ELF_PACKAGE_METADATA/
> 
> If the jansson library is available at build time (and it is explicitly
> enabled), link ld to it, and use it to validate that the input is
> correct JSON, to avoid writing garbage to the file. The
> configure option --enable-jansson has to be used to explicitly enable
> it (error out when not found). This allows bootstrappers (or others who
> are not interested) to seamlessly skip it without issues.
> ---
> v2: fix style issues
>     fix ASAN issue: note storage is rounded up, but that size
>     was used to read the json input, which is fixed
>     add entry to NEWS
>     add note about json validation to docs
> v3: switch libjansson support to disabled-by-default even if
>       the library is available in the build environment
>     update new test to account for old/new readelf, which
>       might or might not pretty-print the FDO note
>     update bootstrap test to link with jansson if the object
>       files being linked were built with it
>     remove dead code

I'm about to commit this with a few formatting fixes, and with a
working bootstrap.exp change.  That one was obviously not tested.  The
idea was good but ld config.h is not in $srcdir/.., and besides that
it's on the build machine not the host (*), and plain grep is more
likely to exist than fgrep.

*) build vs host doesn't matter much in bootstrap.exp since those
tests are only run when native, but I'd like to get it correct for
someone doing copy/paste for a future config.h test.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH v3] ld: add --package-metadata

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2468 bytes --]

On Thu, 2022-05-26 at 13:25 +0930, Alan Modra wrote:
> On Wed, May 25, 2022 at 02:41:47PM +0100, luca.boccassi@gmail.com wrote:
> > From: Luca Boccassi <bluca@debian.org>
> > 
> > Generate a .note.package FDO package metadata ELF note, following
> > the spec: https://systemd.io/ELF_PACKAGE_METADATA/
> > 
> > If the jansson library is available at build time (and it is explicitly
> > enabled), link ld to it, and use it to validate that the input is
> > correct JSON, to avoid writing garbage to the file. The
> > configure option --enable-jansson has to be used to explicitly enable
> > it (error out when not found). This allows bootstrappers (or others who
> > are not interested) to seamlessly skip it without issues.
> > ---
> > v2: fix style issues
> >     fix ASAN issue: note storage is rounded up, but that size
> >     was used to read the json input, which is fixed
> >     add entry to NEWS
> >     add note about json validation to docs
> > v3: switch libjansson support to disabled-by-default even if
> >       the library is available in the build environment
> >     update new test to account for old/new readelf, which
> >       might or might not pretty-print the FDO note
> >     update bootstrap test to link with jansson if the object
> >       files being linked were built with it
> >     remove dead code
> 
> I'm about to commit this with a few formatting fixes, and with a
> working bootstrap.exp change.  That one was obviously not tested.  The
> idea was good but ld config.h is not in $srcdir/.., and besides that
> it's on the build machine not the host (*), and plain grep is more
> likely to exist than fgrep.
> 
> *) build vs host doesn't matter much in bootstrap.exp since those
> tests are only run when native, but I'd like to get it correct for
> someone doing copy/paste for a future config.h test.

Thanks for fixing it! I did test it, and it was working on my machine,
both with and without the new build flag:

Running /home/bluca/git/binutils-gdb/ld/testsuite/ld-bootstrap/bootstrap.exp ...
Running /home/bluca/git/binutils-gdb/ld/testsuite/ld-bpf/bpf.exp ...

I imagine this is because I was running 'make check' from the ld/
directory? I took '$srcdir' and 'fgrep' from existing scripts in other
tests in ld/testsuite. Anyway, the important thing is that it's all
good now.

Thanks again!

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] ld: add --package-metadata

[Florian Weimer]

* Luca Boccassi:

> On Wed, 2022-05-25 at 10:45 +0200, Florian Weimer wrote:
>> * Luca Boccassi:
>> 
>> > So with that experience in the bag, the most obvious next step is to
>> > have a first-class option, that allows a self-contained flag to be
>> > passed instead. After all the idea is similar to the build-id, and
>> > there we have a first-class option too, and it works well.
>> 
>> Maybe it's better to just pre-allocate space for the program header note
>> (and corresponding data) and patch the actual contents in later, maybe
>> as part of the debuginfo post-processing.
>> 
>> This would also side-step any shell encoding issues of the JSON object.
>
> I'm not sure - that still requires to do a lot of the work here, while
> also leaving each and every distro builder to implement a whole load of
> integration on their own, risking divergence to appear at some point,
> while allowing for many more things to go wrong at multiple stages.

But there are far fewer distributions and ELF post-processing mechanisms
than upstream build systems.  (The RPM mechanisms are even shared by
multiple RPM-based distributions which are otherwise quite dissimilar.)

If the JSON is passed as a shell argument, you have to worry about
encoding backslashes and double quotes.  When constructing the string,
you really have to know whether it will be subject to shell parsing or
not.  The current CFLAGS and LDFLAGS we have in the distribution do not
have this issue because they do not contain any shell metacharacters, so
we do not have sufficient experience with this.

I guess you could side-step this with optional base64 encoding.

> The advantage of having a common, standard and shared interface is
> that every distro that opts-in will produce the same results,

You have to get it through the package build system, though.  That's
easier with short, fixed option strings, strings which do not contain
any funny characters.

>> A really nice approach would require changes to the way we generate
>> coredumps: teach the dumper to copy non-allocated sections from a file
>> region.  Then we wouldn't have to pre-allocate section contents at all.
>> I think the core dumper would have to look at section headers for this,
>> not program headers.  We could add a program header that describes the
>> file region to be copied, but it would get out of sync with the file
>> contents if ELF editing tools don't know that it has to be updated if
>> non-allocated sections are changed.
>
> Requiring such sweeping changes in the kernel (it would be a default
> behaviour change, whether one opts in to this or not), which means they
> need to be propagated everywhere, would take years at best, even if it
> was possible, acceptable and it were to actually happen. Also, the
> consumer implementation would no longer be compatible with what we have
> published now in production in CBL-Mariner and Fedora.

Fedora will switch to a different scheme if a better one becomes
available.  Even the present feature itself is redundant relative to
Fedora's build IDs, and clearly they haven't blocked implementation of
the new thing.

Thanks,
Florian