Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] Fix longstanding signal handling bug with improperly set DS register #2191

Merged
merged 2 commits into from
Jan 20, 2025

Conversation

ghaerr
Copy link
Owner

@ghaerr ghaerr commented Jan 20, 2025

After looking at how ia16-elf-gcc and OWC compilers generate some code reloading DS for fast execution when using far pointers, it occurred to me that a ELKS program might be interrupted by the hardware timer or keyboard when the process's DS != SS.

If, on return to execution from an interrupt a signal has been raised for the process and a C signal handler routine registered through signal, the kernel sets up the stack to return to the signal handler, rather than the interrupted normal C code. When this happens, the C library signal handling callback code saves all the registers and calls the C interrupt handler - but does not reset DS to a valid value for C signal handler which has been compiled expecting the DS == SS, and as a result the system may read/write data incorrectly and a system hang or crash results. This finding explains how on rare occasions ELKS may crash after ^C a process, even though the kernel has no known crash issues.

This PR tests and fixes the above usually rare event. The following test code has been written which demonstrates the problem - the program busy loops calling a function which loads the DS register for far pointer memory access, but also having registered an interrupt handler for SIGINT (^C typed). Without this fix, the system will crash on ^C or two. With the fix, DS is set to SS and the registered interrupt handler executes normally, then resets DS to its original value before returning to the interrupted process.

#include <stdio.h>
#include <signal.h>

#define DS() __extension__ ({             \
        unsigned short _v;                \
        asm volatile ("mov %%ds,%%ax\n"   \
                        :"=a" (_v)        \
                        :                 \
                     );                   \
        _v; })

#define SS() __extension__ ({             \
        unsigned short _v;                \
        asm volatile ("mov %%ss,%%ax\n"   \
                        :"=a" (_v)        \
                        :                 \
                     );                   \
        _v; })

#define _MK_FP(seg,off) ((void __far *)((((unsigned long)(seg)) << 16) | ((unsigned int)(off))))

long __far *p = _MK_FP(2, 4);       /* easy to see DS=2 */
volatile unsigned long l;

void reloadDS(void)
{
    l = *p;     /* generates code to reload DS: lds %ss:p,%bx */
                /*                              mov (%bx),%dx */
}

void sighandler(int signo)
{
    (void)signo;

    printf("DS %04x, SS %04x\n", DS(), SS());
    signal(SIGINT, sighandler);
}

int main(int ac, char **av)
{
    (void)ac; (void)av;

    printf("DS %04x, SS %04x\n", DS(), SS());
    signal(SIGINT, sighandler);
    for (;;)
        reloadDS();
}

The revised C library signal callback routines are updated for ia16-elf-gcc and OpenWatcom C compilers. When we implement signal handling in C86, this fix will also be implemented for that compiler.

@ghaerr ghaerr merged commit 678dd4e into master Jan 20, 2025
1 check passed
@ghaerr ghaerr deleted the sigcb branch January 20, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant