Page MenuHomeFreeBSD

Series of fixes for i386
ClosedPublic

Authored by kib on Aug 23 2022, 5:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 11:31 PM
Unknown Object (File)
Sun, Nov 24, 1:33 PM
Unknown Object (File)
Thu, Nov 14, 4:08 PM
Unknown Object (File)
Oct 23 2024, 11:24 AM
Unknown Object (File)
Sep 24 2024, 3:45 AM
Unknown Object (File)
Sep 19 2024, 10:07 AM
Unknown Object (File)
Sep 18 2024, 4:39 PM
Unknown Object (File)
Sep 4 2024, 12:22 AM

Details

Summary
commit 57b1254317cd498d24ea3098a21c17f5d8da5476
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Mon Aug 22 02:14:02 2022 +0300

    fork_exit(): style comment
    
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit 8087742a59d1b780687ff5ca95efca57d1fd96da
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Sun Aug 21 04:47:11 2022 +0300

    x86: improve machdep.uprintf_signal
    
    Print %eax/%rax.
    Use better format strings, like %#x.
    
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit 988d245b2ba96635eae0c2d3bb0c2c56c66b2940
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Sun Aug 21 04:46:13 2022 +0300

    x86: print trap name in addition of trap number
    
    for the "trap with interrupts disabled" warning.
    
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit 7db6274e3c971621a6a842292fa5a8deffcabe78
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Thu Aug 18 07:16:09 2022 +0300

    i386: print all GPRs, PSL, and CR3 on double fault
    
    Also compactify the printfs, and remove comment about 'two prints'.
    Their arguments are on same page, so one fault implies another.
    
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit 7c074ea047f46f510a30f38be37e33bf56486d41
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Mon Aug 22 08:56:45 2022 +0300

    i386: simplify flow control in irettraps
    
    It is enough to have only one 'call calltrap' locally.
    
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit b081fe789db2867fd0f50eb756fb559da2aa3082
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Thu Aug 18 07:07:29 2022 +0300

    i386 doreti: stop saving/restoring %ecx around calls into C
    
    There is no reason to do this.  Instead just calculate it later.
    
    Reported and tested by: pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit c52767a786275985ba883fa62624db2fa5f54e93
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Fri Aug 19 04:26:37 2022 +0300

    i386: do not allow userspace to set tf_trapno on sigreturn(2)
    
    tf_trapno is checked on return from interrupt/exception to determine if
    special handling is needed for switching address space.  This is due to
    the possibility of NMI/MCHK/DBG to occur at arbitrary place in kernel,
    where both address space and stack used could be transient.  Kernel
    saves current %cr3 in tf_err for such events, to restore on return.
    
    If user is able to set tf_trapno, it can trigger that special handling,
    and since tf_err is also user-controlled by sigreturn(2), the result is
    undefined.
    
    PR:     265889
    Reported by:    lwhsu
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit 555094d533abed9bed6036876d7661d2772056c2
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Mon Aug 22 04:20:28 2022 +0300

    irettraps: i386 does not push %ss/%esp when exception does not switch rings
    
    Which means that we must not copy top 8 bytes from the trampoline stack
    for the exception frame to the regular thread kstack.  As consequence,
    this stops corruption of the pcb.  The visible effect was often a broken
    fork(2) on the CPU where corruption occured.
    
    Account for the detail by substracting 8 from the copy byte count when
    moving exception frames from trampoline to the regular stack.
    
    [irettraps handles segmentation/stack/protection faults which could
    occur on the doreti path, where we might already switched stack and
    address space]
    
    Reported and tested by: pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

commit 08950e45cc059ec64bcd64e21cb1b569b641c72d
Author: Konstantin Belousov <kib@FreeBSD.org>
Date:   Tue Aug 9 03:56:54 2022 +0300

    i386 copyout_fast: improve detection of a fault on accessing userspace
    
    Do not blindly account a page fault occuring on the trampoline area,
    as the userspace access fault.  Check that it occured exactly in the
    instruction that does that.
    
    This avoids unneeded switches of address space on faults not needing the
    switch, effectively converting machine resets due to tripple faults,
    into regular panics.
    
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36302

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.Aug 23 2022, 5:21 AM

These all look good to my eye

This revision is now accepted and ready to land.Aug 23 2022, 4:51 PM

The log message for tf_trapno I didn't quite understand at first because I read the diff of osigreturn first and it doesn't actually restore the user values to tf_trapno and tf_err. We could perhaps just leave off the osigreturn change off?

sys/i386/i386/exception.s
233
sys/i386/i386/exec_machdep.c
643 ↗(On Diff #109701)

I guess it is just this one that seems spurious? We don't do a full bcopy() here so the user version of tf_trapno and tf_err is ignored.

kib marked 2 inline comments as done.Aug 24 2022, 3:08 AM
kib added inline comments.
sys/i386/i386/exec_machdep.c
643 ↗(On Diff #109701)

I understand what you are saying, yes, it looks like a spurious change at first. But if we do not override the tf_trapno value in td_frame, then the previous value is going to be used (td_frame is type-stable most of the time). And the previous value might be from the bad list (T_TRCTRAP etc).

This is why I put it there as well.

kib marked an inline comment as done.

Grammar fix

This revision now requires review to proceed.Aug 24 2022, 3:08 AM
This revision is now accepted and ready to land.Aug 24 2022, 6:40 PM