Page MenuHomeFreeBSD

Add a link to the Elf_Brandinfo into the struct sysentvec.
ClosedPublic

Authored by dchagin on Jun 27 2021, 3:30 PM.

Details

Summary

To allow the ABI to make a dicision based on the Brandinfo add a link
to the Elf_Brandinfo into the struct sysentvec. Add a note that the high 8 bits
of Elf_Brandinfo flags is private to the ABI.

This can be done by adding a private_flag filed to the struct sysentvec,
but it requires a lot of more work - duplicate of sysentvec, add some fileds
to the struct sysentvec which is private to the ABI and so on.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 40129
Build 37018: arc lint + arc unit

Event Timeline

dchagin added reviewers: kib, markj.
dchagin changed the repository for this revision from rS FreeBSD src repository - subversion to R10 FreeBSD src repository.
sys/kern/imgact_elf.c
1174

How could this work? Imagine you have two brands pointing to the same sysent. Then the latest executed brand would always override brand_info in sysvec.

sys/kern/imgact_elf.c
1174

How could this work? Imagine you have two brands pointing to the same sysent. Then the latest executed brand would always override brand_info in sysvec.

whoops, indeed. thanks. ugh, i need a knowledge about which is brand is in use, but the second way that i see is not good.

sys/kern/imgact_elf.c
1174

Store the brand in struct proc?

sys/sys/proc.h
737 ↗(On Diff #91413)

This should be named like p_elf_brand_info.
Also, I believe it should be in the section that is copied on fork.
Also, add a proper locking annotation for the new field (the (X) note in other comments).

asserts fixed, tinderboxed

sys/sys/proc.h
706 ↗(On Diff #91418)

You do not clear it on exec, so the note about NULL for non-elf binaries is not true. I am not sure if this needs fixing.

Also note the spelling, it is ELF, not Elf.

s/Elf/ELF/, I think p_elf_brandinfo should be saved and cleared before img activator
work, and restored on error like p_osrel.

This revision is now accepted and ready to land.Jun 28 2021, 11:03 AM
markj added inline comments.
sys/sys/imgact_elf.h
91

The indentation looks a bit strange here.

sys/sys/proc.h
703 ↗(On Diff #91423)

I think p_elf_machine is redundant now, isn't it always equal to p_elf_brandinfo->machine? Maybe not worth addressing in this review.

sys/sys/imgact_elf.h
91

the same as above, see Elf_Brandnote, in the same style

sys/sys/proc.h
703 ↗(On Diff #91423)

yes, it is equal, this will be the next

Why would we want ABI to do this, though?

Why would we want ABI to do this, though?

for now its https://reviews.freebsd.org/D30332