public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Giuliano Procida <gprocida@google.com>
To: libabigail@sourceware.org
Cc: dodji@seketeli.org, kernel-team@android.com, gprocida@google.com,
	 maennich@google.com, sidnayyar@google.com, vvvvvv@google.com
Subject: [PATCH] Narrow Linux symbol CRCs to 32 bits
Date: Thu, 27 Oct 2022 15:09:28 +0100	[thread overview]
Message-ID: <20221027140928.1480353-1-gprocida@google.com> (raw)

MODVERSIONS CRCs are 32-bit hashes of strings representing C type
elements or typed symbols. The hash is calculated using a 32-bit CRC,
hence the name. The kernel module loading code (implicitly) truncates
any provided CRC value to 32 bits before comparing it with anything.

When support was added to libabigail, values up to 64 bits wide were
supported. Recently, Linux kernel builds have now started generating
ELF CRC symbols with 64-bit values (where the low 32 bits are the
CRC). Together this has resulted in incorrect CRCs in ABIs.

This change resolves the problem by narrowing libabigail's concept of
Linux CRC to 32 bits. No tests are affected.

	* include/abg-ir.h (elf_symbol::elf_symbol): Change CRC type
	from optional<uint64_t> to optional<uint32_t>.
	(elf_symbol::create): Likewise.
	(elf_symbol::get_crc): Likewise.
	(elf_symbol::set_crc): Likewise.
	* src/abg-ir.cc (elf_symbol::priv) Change CRC type from
	optional<uint64_t> to optional<uint32_t>.
	(elf_symbol::priv::priv): Likewise.
	(elf_symbol::elf_symbol): Likewise.
	(elf_symbol::create): Likewise.
	(elf_symbol::get_crc): Likewise.
	(elf_symbol::set_crc): Likewise.
	* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
	Change CRC type from optional<uint64_t> to
	optional<uint32_t>.
	* src/abg-symtab-reader.cc (symtab::load_): Change crc_values
	value type from uint64_t to uint32_t.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 include/abg-ir.h         |  8 ++++----
 src/abg-ir.cc            | 12 ++++++------
 src/abg-reporter-priv.cc |  4 ++--
 src/abg-symtab-reader.cc |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/abg-ir.h b/include/abg-ir.h
index 4892f0e2..546603f4 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -940,7 +940,7 @@ private:
 	     const version&	ve,
 	     visibility		vi,
 	     bool		is_in_ksymtab = false,
-	     const abg_compat::optional<uint64_t>&	crc = {},
+	     const abg_compat::optional<uint32_t>&	crc = {},
 	     const abg_compat::optional<std::string>&	ns = {},
 	     bool		is_suppressed = false);
 
@@ -966,7 +966,7 @@ public:
 	 const version&	    ve,
 	 visibility	    vi,
 	 bool		    is_in_ksymtab = false,
-	 const abg_compat::optional<uint64_t>&		crc = {},
+	 const abg_compat::optional<uint32_t>&		crc = {},
 	 const abg_compat::optional<std::string>&	ns = {},
 	 bool		    is_suppressed = false);
 
@@ -1039,11 +1039,11 @@ public:
   void
   set_is_in_ksymtab(bool is_in_ksymtab);
 
-  const abg_compat::optional<uint64_t>&
+  const abg_compat::optional<uint32_t>&
   get_crc() const;
 
   void
-  set_crc(const abg_compat::optional<uint64_t>& crc);
+  set_crc(const abg_compat::optional<uint32_t>& crc);
 
   const abg_compat::optional<std::string>&
   get_namespace() const;
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 5193934a..a043a532 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1788,7 +1788,7 @@ struct elf_symbol::priv
   //     STT_COMMON definition of that name that has the largest size.
   bool			is_common_;
   bool			is_in_ksymtab_;
-  abg_compat::optional<uint64_t>	crc_;
+  abg_compat::optional<uint32_t>	crc_;
   abg_compat::optional<std::string>	namespace_;
   bool			is_suppressed_;
   elf_symbol_wptr	main_symbol_;
@@ -1822,7 +1822,7 @@ struct elf_symbol::priv
        const elf_symbol::version& ve,
        elf_symbol::visibility	  vi,
        bool			  is_in_ksymtab,
-       const abg_compat::optional<uint64_t>&	crc,
+       const abg_compat::optional<uint32_t>&	crc,
        const abg_compat::optional<std::string>&	ns,
        bool			  is_suppressed)
     : env_(e),
@@ -1896,7 +1896,7 @@ elf_symbol::elf_symbol(const environment* e,
 		       const version&	  ve,
 		       visibility	  vi,
 		       bool		  is_in_ksymtab,
-		       const abg_compat::optional<uint64_t>&	crc,
+		       const abg_compat::optional<uint32_t>&	crc,
 		       const abg_compat::optional<std::string>&	ns,
 		       bool		  is_suppressed)
   : priv_(new priv(e,
@@ -1971,7 +1971,7 @@ elf_symbol::create(const environment* e,
 		   const version&     ve,
 		   visibility	      vi,
 		   bool		      is_in_ksymtab,
-		   const abg_compat::optional<uint64_t>&	crc,
+		   const abg_compat::optional<uint32_t>&	crc,
 		   const abg_compat::optional<std::string>&	ns,
 		   bool		      is_suppressed)
 {
@@ -2208,7 +2208,7 @@ elf_symbol::set_is_in_ksymtab(bool is_in_ksymtab)
 /// Getter of the 'crc' property.
 ///
 /// @return the CRC (modversions) value for Linux Kernel symbols, if any
-const abg_compat::optional<uint64_t>&
+const abg_compat::optional<uint32_t>&
 elf_symbol::get_crc() const
 {return priv_->crc_;}
 
@@ -2216,7 +2216,7 @@ elf_symbol::get_crc() const
 ///
 /// @param crc the new CRC (modversions) value for Linux Kernel symbols
 void
-elf_symbol::set_crc(const abg_compat::optional<uint64_t>& crc)
+elf_symbol::set_crc(const abg_compat::optional<uint32_t>& crc)
 {priv_->crc_ = crc;}
 
 /// Getter of the 'namespace' property.
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 2ecf8016..70839085 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -1149,8 +1149,8 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr&	symbol1,
 	  << "\n";
     }
 
-  const abg_compat::optional<uint64_t>& crc1 = symbol1->get_crc();
-  const abg_compat::optional<uint64_t>& crc2 = symbol2->get_crc();
+  const abg_compat::optional<uint32_t>& crc1 = symbol1->get_crc();
+  const abg_compat::optional<uint32_t>& crc2 = symbol2->get_crc();
   if (crc1 != crc2)
     {
       const std::string none = "(none)";
diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
index 606c96a2..8a566f8d 100644
--- a/src/abg-symtab-reader.cc
+++ b/src/abg-symtab-reader.cc
@@ -265,7 +265,7 @@ symtab::load_(Elf*	       elf_handle,
 
   const bool is_kernel = elf_helpers::is_linux_kernel(elf_handle);
   std::unordered_set<std::string> exported_kernel_symbols;
-  std::unordered_map<std::string, uint64_t> crc_values;
+  std::unordered_map<std::string, uint32_t> crc_values;
   std::unordered_map<std::string, std::string> namespaces;
 
   for (size_t i = 0; i < number_syms; ++i)
-- 
2.38.0.135.g90850a2211-goog


             reply	other threads:[~2022-10-27 14:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-27 14:09 Giuliano Procida [this message]
2022-10-27 15:53 ` Giuliano Procida
2022-10-28 15:13   ` Dodji Seketeli
2022-10-28 16:08     ` Giuliano Procida
2022-11-17 10:13       ` Dodji Seketeli
2022-11-11 13:21 ` [PATCH v2] " Giuliano Procida
2022-11-11 15:44   ` Giuliano Procida
2022-11-17 10:10   ` Dodji Seketeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221027140928.1480353-1-gprocida@google.com \
    --to=gprocida@google.com \
    --cc=dodji@seketeli.org \
    --cc=kernel-team@android.com \
    --cc=libabigail@sourceware.org \
    --cc=maennich@google.com \
    --cc=sidnayyar@google.com \
    --cc=vvvvvv@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).