6

I'm using the following code to try to read an input from user and timeout and exit if more than 5 seconds pass. This is accomplished through a combination of setjmp/longjmp and the SIGALRM signal.

Here's the code:

#include <stdio.h>
#include <setjmp.h>
#include <unistd.h>
#include <string.h>
#include <sys/signal.h>

jmp_buf buffer;

// this will cause t_gets() to return -2
void timeout() {
    longjmp(buffer, 1);
}

int t_gets(char* s, int t)
{
    char* ret;
    signal(SIGALRM, timeout);
    if (setjmp(buffer) != 0)
        return -2; // <--- timeout() will jump here
    alarm(t);
    // if fgets() does not return in t seconds, SIGALARM handler timeout()
    // will be called, causing t_gets() to return -2
    ret = fgets(s, 100, stdin);
    alarm(0);
    if (ret == NULL ) return -1;
    return strlen(s);
}

int main()
{
    char s[100];
    int z=t_gets(s, 5);
    printf("%d\n", z); 
}

Now, my question is if there's anything that can go wrong with this function. I've read that calling longjmp() from a signal handler can have undefined behaviour, what exactly is it refferring to?

Also, what if the alarm triggers right after fgets() returns, but before alarm(0) is called? Will it cause the function to return -2 even if the user did input something?

LATER EDIT: I'm not interested in ways to improve the code. I just want to know how it might fail.

sttwister
  • 2,279
  • 1
  • 19
  • 23
  • `ret` isn't initialized, so the call to `fgets` will cause memory corruption. – outis Nov 11 '09 at 14:21
  • @outis: `ret` gets assigned with the result of the `fgets()` function call. – pmg Nov 11 '09 at 14:25
  • fgets() returns the first parameter in case of succes or NULL in case of any error. I don't see how that might cause any memory corruption. The contents are saved in s. – sttwister Nov 11 '09 at 14:25
  • Never mind--skimmed to fast. Misread "fgets(s, ...)". – outis Nov 11 '09 at 14:31

7 Answers7

8

From the man page for longjmp:

POSIX does not specify whether longjmp() will restore the signal context. If you want to save and restore signal masks, use siglongjmp()

Your second question: Yes, the function will return -2 because longjmp() will cause it to go to the setjmp(buffer) part, but the timing will have to be very precise.

Wernsey
  • 5,411
  • 22
  • 38
  • It doesn't matter if the signal context is restored or not since calling `longjmp()` will cause the function to return and no more signals will be necessary for the alarm. – sttwister Nov 11 '09 at 14:38
  • A note, regular signals (i.e. SIGUSR*) are _combined_ by the kernel when they fall in line. RT signals are delivered as they come. This means, you broke a primitive but effective messaging scheme using longjmp(), especially if the thread is in some kind of I/O sleep (voluntary, or D sleep). – Tim Post Nov 11 '09 at 15:51
  • This is true in your specific program, because the function returns directly to `main` which exits. The fact that a function has an unwanted side effect of clobbering the signal mask is a problem in general. – Kaz Jul 08 '16 at 15:24
2

When it comes to what could go wrong when behavior is undefined, the only answer you can count on is "anything, including nothing". Maybe nothing goes wrong, maybe you get a segfault, maybe you get nasal daemons.

More specific answers depend on which system and release you're working with. For example, on Linux distros (at least, all since 2000), the kernel performs some tasks after the signal handler returns. If you longjmp, you'll probably leave junk on the kernel stack that could cause problems later on, such as erroneously returning to the code your program was executing when the signal was caught (the call to 'fgets' in the example). Or not.

Calling longjmp within a signal handler can also (in general, but probably not in your example) introduce a security hole.

outis
  • 75,655
  • 22
  • 151
  • 221
  • 2
    The CERT article, like most of their "secure coding" articles, it slightly wrong. It's safe to call `longjmp` from a signal handler **if and only if** you can ensure that the signal handler does not interrupt an async-signal-unsafe function. Unfortunately `fgets` is async-signal-unsafe, so OP's code is bogus. – R.. GitHub STOP HELPING ICE Aug 16 '11 at 02:52
2

Another nice (or ugly depending on your perspective) way to make fgets bail out would be:

int tmp = dup(0);
ret = fgets(s, 100, stdin);
if (!ret && errno == EBADF) clearerr(stdin);
dup2(tmp, 0);
close(tmp);

And in the signal handler:

close(0);

Presumably this works even on ancient systems without sigaction and with BSD signal semantics.

R.. GitHub STOP HELPING ICE
  • 208,859
  • 35
  • 376
  • 711
1

I don't think you need to use setjmp/longjmp. fgets should be interrupted by signals (errno is set to EINTR), though you'll probably need to use sigaction(...) rather than signal(...) to ensure SA_RESTART is clear.

void timeout(int) {
   // doesn't actually need to do anything
}
int t_gets(char* s, int t)
{
    char* ret;
    struct sigaction action = {0};
    action.sa_handler = timeout;
    sigaction(SIGALRM, &action, NULL);
    alarm(t);
    // if fgets() does not return in t seconds, SIGALARM handler timeout()
    // will be called, interrupting fgets and causing t_gets() to return -2
    ret = fgets(s, 100, stdin);
    // even if the alarm is called after fgets returns, it won't erroneously cause
    // t_gets to return -2
    int err = errno;
    alarm(0);
    if (ret == NULL) { 
        switch (err) {
        case EINTR:
            return -2;
        // add other cases as warranted
        default:
            return -1;
        }
    }
    return strlen(s);
}
outis
  • 75,655
  • 22
  • 151
  • 221
  • `fgets()` should fail after `t` seconds, regardless whether any signal was sent. – sttwister Nov 11 '09 at 14:33
  • I don't see how `fgets` can fail without sending a signal. Aren't you using the alarm signal to cause failure? – outis Nov 11 '09 at 14:48
  • Indeed, using `sigaction` does actually cause `fgets` to fail. Thanks for your answer. However (if you also read my later edit), I'm more interested in ways the approach I posted might fail. – sttwister Nov 11 '09 at 14:56
1

You can replace the longjmp/setjmp with siglongjmp/sigsetjump and then won't have the issue of the signal context being undefined after the jmp. You may not really care here since you aren't explicitly changing the mask. I forget if the mask is altered by signal call itself.

What is perhaps the bigger issue is ensuring that your code is signal safe. For example, does fgets() acquire any mutex (perhaps implicitly as a part of a malloc call)? If it is and your timer goes off while that mutex is held, your program is toast next time you try to do a heap allocation.

Peeter Joot
  • 7,848
  • 7
  • 48
  • 82
0

About your second question you can add a lock that blocks the return -2 when the main thread has passed the fgets call.

Ignacio Soler Garcia
  • 21,122
  • 31
  • 128
  • 207
0

what if the alarm triggers right after fgets() returns, but before alarm(0) is called?

You could initialize ret (to NULL, possibly) and check that in the body of the if(setjmp()) statement:

/* NOT TESTED */
int t_gets(char* s, int t)
{
    char* ret = NULL;
    signal(SIGALRM, timeout);
    if (setjmp(buffer) != 0) {
        // timeout() will jump here
        if (ret == NULL) {
            return -2;
        } else {
            goto end_of_function;
        }
    }
    alarm(t);
    // if fgets() does not return in t seconds, SIGALARM handler timeout()
    // will be called, causing t_gets() to return -2
    ret = fgets(s, 100, stdin);
end_of_function:
    alarm(0);
    if (ret == NULL ) return -1;
    return strlen(s);
}
pmg
  • 106,608
  • 13
  • 126
  • 198