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 40135
Build 37024: 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
1173

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
1173

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
1173

Store the brand in struct proc?

sys/sys/proc.h
738

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
705

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
92

The indentation looks a bit strange here.

sys/sys/proc.h
703

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
92

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

sys/sys/proc.h
703

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