[PATCH 0/1] Fix abi mismatch error on boot for cross-compiled images

  • Open
  • quality assurance status badge
Details
2 participants
  • Christoph Buck
  • Ludovic Courtès
Owner
unassigned
Submitted by
Christoph Buck
Severity
normal
C
C
Christoph Buck wrote 4 days ago
(address . guix-patches@gnu.org)(name . Christoph Buck)(address . dev@icepic.de)
cover.1731253918.git.dev@icepic.de
Hi!

The following patch fixes the `record-abi-mismatch-error` early on boot during
execution of initrd.cpio.gz if the image was cross-compiled on x64 for an
32bit platform (e.g arm32 or i868).

For a comprehensive analysis see the correspond thread [1] on the guix-help
mailing list.

The root cause of the issue is as follows:

During compilation guix stores a hash of the record field names in the
compiled go files. On runtime this has is recalcuated and checked against the
stored hash to verify that no abi mismatch occured. As described in [1] this
hash differs if the corresponding record was compiled in a cross-compiled
context. Guile uses internally an `unsigned long` to store the hash, which
results in hashes of different sizes depending on the platform the guile
compiler is executed on. Guix already tries to work around this problem by
limiting the size of the hash in a cross-compile context to the most positive
fixnum size of the target, but this is insufficient, because, as one can look
up in the guile source code, the size is limited by an modulo operation after
the hash was already calculated for an 8byte unsigned long. Therefore the
resulting hashes during compilation and execution are different and an abi
mismatch error is erroneously reported during runtime.

An easy workaround is documented in the guile src namely in an comment of the
`JENKINS_LOOKUP3_HASHWORD2`, which is used to calculate the hash:

Toggle quote (5 lines)
> Scheme can access symbol-hash, which exposes this value. For
>cross-compilation reasons, we ensure that the high 32 bits of the hash on a
>64-bit system are equal to the hash on a 32-bit system. The low 32 bits just
>add more entropy.

This suggest the following workaround. Always limit the hash size to 32bit
even if executed on a 64bit platform (or to be more specific a platform where
ulong is 8bytes big). Do this by right shift the hash value 32bits and don't
rely on the size parameter of the `string-hash` function. This is what this
patch tries to accomplish.

Imho this approach has two drawbacks. Lost entropy on 64 bit machines and the
abi break because on new compilation the hash values on 64bit platforms will
change. The lost entropy is irrelevant because the hash is not used in an
cryptophically relevant context. For the abi break i am not sure how severe
this change is.


[1] ABI mismatch on boot on arm32 system



Christoph Buck (1):
guix: records: Fix abi check in cross compile context

guix/records.scm | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)


base-commit: 2a6d96425eea57dc6dd48a2bec16743046e32e06
--
2.45.1
C
C
Christoph Buck wrote 4 days ago
[PATCH 1/1] guix: records: Fix abi check in cross compile context
(address . 74296@debbugs.gnu.org)(name . Christoph Buck)(address . dev@icepic.de)
c31ee4b87114ec5785854395991bc46293d0202e.1731253918.git.dev@icepic.de
* guix/records.scm: Use 32bit hash value for abi check to prevent
`record-abi-mismatch-error` in a cross-compile context

Change-Id: I889747b1a2837bee8bf9b4de5729fdcf1b165380
---
guix/records.scm | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

Toggle diff (25 lines)
diff --git a/guix/records.scm b/guix/records.scm
index dca1e3c2e7..2084499e36 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -415,11 +415,13 @@ (define-syntax define-record-type*
;; list of symbols.
(syntax-case field-specs ()
(((field get properties ...) ...)
- (string-hash (object->string
- (syntax->datum #'((field properties ...) ...)))
- (cond-expand
- (guile-3 (target-most-positive-fixnum))
- (else most-positive-fixnum))))))
+ (let ((hash-value
+ (string-hash
+ (object->string (syntax->datum #'((field properties ...) ...))))))
+ (cond
+ ((< most-positive-fixnum (ash 1 32)) hash-value)
+ ((< most-positive-fixnum (ash 1 64)) (ash hash-value -32))
+ (else (error "unexpected!" most-positive-fixnum)))))))
(syntax-case s ()
((_ type syntactic-ctor ctor pred
--
2.45.1
L
L
Ludovic Courtès wrote 2 days ago
Re: [bug#74296] [PATCH 0/1] Fix abi mismatch error on boot for cross-compiled images
(name . Christoph Buck)(address . dev@icepic.de)
87o72krs8n.fsf@gnu.org
Hi Christoph,

Christoph Buck <dev@icepic.de> skribis:

Toggle quote (28 lines)
> During compilation guix stores a hash of the record field names in the
> compiled go files. On runtime this has is recalcuated and checked against the
> stored hash to verify that no abi mismatch occured. As described in [1] this
> hash differs if the corresponding record was compiled in a cross-compiled
> context. Guile uses internally an `unsigned long` to store the hash, which
> results in hashes of different sizes depending on the platform the guile
> compiler is executed on. Guix already tries to work around this problem by
> limiting the size of the hash in a cross-compile context to the most positive
> fixnum size of the target, but this is insufficient, because, as one can look
> up in the guile source code, the size is limited by an modulo operation after
> the hash was already calculated for an 8byte unsigned long. Therefore the
> resulting hashes during compilation and execution are different and an abi
> mismatch error is erroneously reported during runtime.
>
> An easy workaround is documented in the guile src namely in an comment of the
> `JENKINS_LOOKUP3_HASHWORD2`, which is used to calculate the hash:
>
>> Scheme can access symbol-hash, which exposes this value. For
>>cross-compilation reasons, we ensure that the high 32 bits of the hash on a
>>64-bit system are equal to the hash on a 32-bit system. The low 32 bits just
>>add more entropy.
>
> This suggest the following workaround. Always limit the hash size to 32bit
> even if executed on a 64bit platform (or to be more specific a platform where
> ulong is 8bytes big). Do this by right shift the hash value 32bits and don't
> rely on the size parameter of the `string-hash` function. This is what this
> patch tries to accomplish.

Woow, thanks for the investigation & explanation!

(I would say that the ‘scm_ihash’ implementation as a mere modulo is
dubious, but that’s hard to change anyway.)

Toggle quote (6 lines)
> Imho this approach has two drawbacks. Lost entropy on 64 bit machines and the
> abi break because on new compilation the hash values on 64bit platforms will
> change. The lost entropy is irrelevant because the hash is not used in an
> cryptophically relevant context. For the abi break i am not sure how severe
> this change is.

Capping at 32-bits means that potentially some ABI changes could go
unnoticed, but that’s extremely unlikely if the hash function is good
enough.

I believe the ABI break is fine too: developers will have to
“make clean-go && make”, but that’s okay.

Thoughts? Opinions?

Ludo’.
C
C
Christoph Buck wrote 34 hours ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
87iksrpaiq.fsf@icepic.de
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (2 lines)
> Woow, thanks for the investigation & explanation!

Your are welcome :) I usually keep notes during investigation of bugs
and append them to my patches. This keeps my train of thought
transparent and makes it easier for others to follow along or spot
obvious errors on my side. However, it can get a little bit noisy. So
let met now if i should "keep it done" more.

Toggle quote (4 lines)
> Capping at 32-bits means that potentially some ABI changes could go
> unnoticed, but that’s extremely unlikely if the hash function is good
> enough.

Yes, but this problem exits for 32bit builds in general.

Toggle quote (3 lines)
> I believe the ABI break is fine too: developers will have to
> “make clean-go && make”, but that’s okay.

Good to know.

--
Best regards

Christoph
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 74296@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 74296
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch