Skip to content

Support read-only section for curve25519_x25519base_byte.S#247

Open
VlkrS wants to merge 1 commit into
awslabs:mainfrom
VlkrS:aarch64-rodata
Open

Support read-only section for curve25519_x25519base_byte.S#247
VlkrS wants to merge 1 commit into
awslabs:mainfrom
VlkrS:aarch64-rodata

Conversation

@VlkrS
Copy link
Copy Markdown

@VlkrS VlkrS commented Jun 26, 2025

The attached patch permits the read-only section to be enabled. Tested via aws-lc-sys -> deno on OpenBSD 7.7-current.

Related to: #174 #207

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

The attached patch permits the read-only section to be enabled.
Tested via aws-lc-sys -> deno on OpenBSD 7.7-current.
@jargh
Copy link
Copy Markdown
Contributor

jargh commented Jun 26, 2025

The code change looks good, but we'll need to update the proof accordingly. We now support .rodata sections in the proof infrastructure (see 471250b and 15be434) so it should be possible to do this, possibly with some more minor code changes. Are you happy if we build this on your PR?

@VlkrS
Copy link
Copy Markdown
Author

VlkrS commented Jun 27, 2025

Absolutely, please do!

As a side note: enabling the read-only section on the amd64 equivalent works without modification in the same setup.


adr x10, curve25519_x25519base_byte_edwards25519_0g
adr x11, curve25519_x25519base_byte_edwards25519_8g
adrp x10, curve25519_x25519base_byte_edwards25519_0g
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AIUI page-level relocations for mach-o require @PAGE on the end of this, and @PAGEOFF on the end of the add operand (which also needs to lose the ELF-specific :lo12: relocation designation.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other possibility is just requiring these to be page-aligned, which means we can lose the add adjustments and only need to support one kind of relocation specifier. That's what I did over here (macro def for reference).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed Mach-O is a bit different. I think, after a bit of testing, that the following works with both Linux generating ELF and Mac OS generating Mach-O. (I just use ..._data since I think it will simplify the proof if we just have a single relocated symbol for the start of the data.) I'm confident we can prove this since I've prototyped something similar. But I suspect it needs a bit more refinement for other platforms. I'm hoping that if we delete the linux part of the ifdef it would work on OpenBSD and indeed other BSDs - does that look reasonable? (tab is just cpp'd to a register).

#if defined(__linux__) && defined(__ELF__)
        adrp    tab, curve25519_x25519base_byte_data
        add     tab, tab, :lo12:curve25519_x25519base_byte_data
#else
        adrp    tab, curve25519_x25519base_byte_data@PAGE
        add     tab, tab, curve25519_x25519base_byte_data@PAGEOFF
#endif


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants