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)
Sat, Sep 21, 4:53 AM
Unknown Object (File)
Sun, Sep 15, 4:29 PM
Unknown Object (File)
Thu, Sep 12, 3:48 PM
Unknown Object (File)
Sun, Sep 8, 11:44 AM
Unknown Object (File)
Mon, Sep 2, 3:11 AM
Unknown Object (File)
Sun, Sep 1, 5:05 AM
Unknown Object (File)
Sat, Aug 31, 11:50 AM
Unknown Object (File)
Mon, Aug 26, 9:51 PM
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
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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