Page MenuHomeFreeBSD

amd64: Relax the assertion added in commit 4a59cbc12
ClosedPublic

Authored by markj on Jun 1 2021, 2:19 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Feb 29, 4:45 PM
Unknown Object (File)
Feb 8 2024, 5:30 PM
Unknown Object (File)
Dec 25 2023, 11:39 PM
Unknown Object (File)
Dec 23 2023, 12:28 PM
Unknown Object (File)
Nov 29 2023, 1:15 AM
Unknown Object (File)
Nov 28 2023, 11:24 AM
Unknown Object (File)
Nov 26 2023, 5:23 AM
Unknown Object (File)
Nov 25 2023, 10:39 AM
Subscribers

Details

Summary

We only need to ensure that interrupts are disabled when handling a
fault from iret. Otherwise it's possible to trigger the assertion
legitimately, e.g., by copying in from an invalid address.

Fixes: 4a59cbc12
Reported by: pho

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 39638
Build 36527: arc lint + arc unit

Event Timeline

markj requested review of this revision.Jun 1 2021, 2:19 PM

Ok, but I think that segment load faults should not have interrupts enabled. Since we do not context-switch them, having indefinite state for any of the segments is probably not too healthy.

This revision is now accepted and ready to land.Jun 1 2021, 2:43 PM
In D30594#687126, @kib wrote:

Ok, but I think that segment load faults should not have interrupts enabled. Since we do not context-switch them, having indefinite state for any of the segments is probably not too healthy.

Yeah, that's why I wrote it this way originally. I missed that we handle pcb_onfault != NULL in the same block.

I can add the same assertion to all of the segment load fault cases. Do you see a better approach?

In D30594#687126, @kib wrote:

Ok, but I think that segment load faults should not have interrupts enabled. Since we do not context-switch them, having indefinite state for any of the segments is probably not too healthy.

Yeah, that's why I wrote it this way originally. I missed that we handle pcb_onfault != NULL in the same block.

I can add the same assertion to all of the segment load fault cases. Do you see a better approach?

No, I think explicitly mark all cases with asserts is the only way. Or it could be a table of addresses and new %rips for segment faults, with one loop iterating over, and one assert in case of the match. But I think it is not needed (yet).

Add assertions for segment register update faults. Convert to using a table.

This revision now requires review to proceed.Jun 1 2021, 5:20 PM

Actually I am having trouble convincing myself that

In D30594#687163, @kib wrote:
In D30594#687126, @kib wrote:

Ok, but I think that segment load faults should not have interrupts enabled. Since we do not context-switch them, having indefinite state for any of the segments is probably not too healthy.

Yeah, that's why I wrote it this way originally. I missed that we handle pcb_onfault != NULL in the same block.

I can add the same assertion to all of the segment load fault cases. Do you see a better approach?

No, I think explicitly mark all cases with asserts is the only way. Or it could be a table of addresses and new %rips for segment faults, with one loop iterating over, and one assert in case of the match. But I think it is not needed (yet).

I converted to a table, it seemed quite ugly to duplicate the assertion for each case. I tested with a program that uses setcontext() to set bogus segment register values.

This revision is now accepted and ready to land.Jun 1 2021, 9:36 PM