amd64: initialize td_frame stack area for init(8) main thread
ClosedPublic
Actions

Authored by kib on Mar 25 2024, 1:55 PM.

Details

Reviewers

markj
andrew
manu
emaste
jhibbits
jhb

Commits

rGe0c92dd2b7e6: amd64: initialize td_frame stack area for init(8) main thread

Summary

Unitialized td_frame mostly does not matter since all registers are
overwritten on exec to activate init(8).  Except PSL_T bit from the
%rflags which might leak into fresh init as garbage, causing spurious
SIGTRAPs delivered to init until first syscall is executed.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Mar 25 2024, 1:55 PM

Herald added a reviewer: andrew. · View Herald TranscriptMar 25 2024, 1:55 PM

Herald added a reviewer: andrew. · View Herald Transcript

Herald added a reviewer: manu. · View Herald Transcript

Herald added subscribers: riscv, olce, jhibbits and 2 others. · View Herald Transcript

kib requested review of this revision.Mar 25 2024, 1:55 PM

Would it be excessive to just zero td_frame in MI code?

In D44498#1014879, @emaste wrote:

Would it be excessive to just zero td_frame in MI code?

I am not sure that this is the correct state of the registers file for all arches.

In D44498#1014883, @kib wrote:

In D44498#1014879, @emaste wrote:

Would it be excessive to just zero td_frame in MI code?

I am not sure that this is the correct state of the registers file for all arches.

Does it not go through exec_setregs() at some point through kern_execve()? That should set all the registers.

In D44498#1014892, @jhibbits wrote:

In D44498#1014883, @kib wrote:

In D44498#1014879, @emaste wrote:

Would it be excessive to just zero td_frame in MI code?

I am not sure that this is the correct state of the registers file for all arches.

Does it not go through exec_setregs() at some point through kern_execve()? That should set all the registers.

Please read the summary which specifically mentions %rflags.PSL_T as the cause of this patch.

In D44498#1014893, @kib wrote:

In D44498#1014892, @jhibbits wrote:

In D44498#1014883, @kib wrote:

In D44498#1014879, @emaste wrote:

Would it be excessive to just zero td_frame in MI code?

I am not sure that this is the correct state of the registers file for all arches.

Does it not go through exec_setregs() at some point through kern_execve()? That should set all the registers.

Please read the summary which specifically mentions %rflags.PSL_T as the cause of this patch.

Sorry, I know nothing about amd64 CPU registers. How is that different from other registers? I would expect all architected per-process registers to be set up in exec_setregs().

Speaking from the PowerPC side, exec_setregs() already bzero()s the trap frame anyway.

Ah, reading exec_setregs() for amd64, I see that it preserves the initial PSL_T. Got it now.

Going back to @emaste's query, it seems to me that bzeroing in MI makes the most sense. IMO exec_setregs(), and the entire MD code path from kern_execve() should assume zero as a 'normal' state, and init should be a 'normal' process from exec standpoint.