Why you should avoid using SIGALRM for timer

I came across a very interesting bug that occurred in a Linux software stack that was ported from some kind of embedded processor back in the days.

The processor back in the days had a probably a single timer interrupt, so the software team decided to implement over it a series of SW timers.

They did something which is quite usual in embedded world, they created a array of active timers, and always set the HW timer interrupt to occur when the nearest SW timer expires.

This requires a bit of a bookkeeping, as we need to dynamically update the HW timer whenever a new “closer” timer is scheduled. and this happens every-time a timer is created, stopped or expire.

We’re working of course on a multi-tasks system that work asynchronously so, a mutex was used to synchronize all of this.

This worked great for embedded. but when the software was ported to Linux. they didn’t want to change the architecture so what they did was to emulate the HW timer interrupt using the alarm signal (SIGALRM)

It worked, it was kept like this for few years. until a bug that was raised from a customer about the system going unresponsive once in a while (could be days) and required reboot.

I asked the customer to crash the application on purpose using SIGABRT when the problems occurs again and send me the coredump.

The precise commands I sent to the client are:

$ ulimit -c unlimited
$ kill -6 $(pidof -s APPNAME)

Once I had the coredump, I analyzed it with gdb and found that there was a blocked call to setitimer()

In the callstack I saw that a thread was requesting a new timer, which called in turn the bookkeeping function that updates the “HW interrupt” to the closest timeout.

I opened LXR and searched for the setitimer syscall implementation in Linux kernel.

Here’s the code for the function:

int do_setitimer(int which, struct itimerval *value, struct itimerval *ovalue)
{
        struct task_struct *tsk = current;
        struct hrtimer *timer;
        ktime_t expires;

        /*
         * Validate the timevals in value.
         */
        if (!timeval_valid(&value->it_value) ||
            !timeval_valid(&value->it_interval))
                return -EINVAL;

        switch (which) {
        case ITIMER_REAL:
again:
                 spin_lock_irq(&tsk->sighand->siglock);
                 timer = &tsk->signal->real_timer;
                 if (ovalue) {
                       ovalue->it_value = itimer_get_remtime(timer);
                       ovalue->it_interval
                                 =
ktime_to_timeval(tsk->signal->it_real_incr);
                }
                /* We are sharing ->siglock with it_real_fn() */
                if (hrtimer_try_to_cancel(timer) < 0) {
                        spin_unlock_irq(&tsk->sighand->siglock);
                        goto again;
                }
                expires = timeval_to_ktime(value->it_value);
                if (expires.tv64 != 0) {
                        tsk->signal->it_real_incr =
                                timeval_to_ktime(value->it_interval);
                        hrtimer_start(timer, expires, HRTIMER_MODE_REL);
                } else
                        tsk->signal->it_real_incr.tv64 = 0;

                trace_itimer_state(ITIMER_REAL, value, 0);
                spin_unlock_irq(&tsk->sighand->siglock);
                break;
        case ITIMER_VIRTUAL:
                set_cpu_itimer(tsk, CPUCLOCK_VIRT, value, ovalue);
                break;
        case ITIMER_PROF:
                set_cpu_itimer(tsk, CPUCLOCK_PROF, value, ovalue);
                break;
        default:
                return -EINVAL;
        }
        return 0;
}

Pay a close lock at line: 17. the call to spin_lock_irq()
This is the only place we can get stuck in this syscall. but why are we blocked, why we can’t get a lock to siglock ?

I grep’ed the kernel code for siglock, who else takes it, and it appears that it is used when a signal is delivered to a user process.
It is kept locked until the user signal handler function returns… (you can read the code here.

OK. so it appears that a signal handler was running alongside of the setitimer() function call.
Let’s review the code

void sig_timer_handler(int signo) {
  pthread_mutex_lock(&g_timer_mutex);
  ..
  ..
  pthread_mutex_unlock(&g_timer_mutex);
}

void do_timer_bookkeeping(void) {
  pthread_mutex_lock(&g_timer_mutex);
  ..
  setitimer(...);
  ..
  pthread_mutex_unlock(&g_timer_mutex);
}

Do you see the problem ????

The problem occurs when the signal function is triggered just before we call setitimer().
The signal handler blocks, as the mutex is currently being held by do_timer_bookkeeping() and this is the deadlock.

OK.

So what’s wrong here ? besides using signals in the first place ?(I think I heard someone says signals are evil)
What’s wrong is that there are a limited set of functions that you can call while you’re in a signal handler, and ptherad_mutex_lock() is not a part of that list.
Whenever you call function not in the list (you can find the list here) the outcome is undefined and subject to implementation.

To fix it, I removed all the SIGALRM code and replaced it with setitimer.
I set the previous SIGALRM handler as the thread that will be spawned whenever the timer expired.

This allowed me to copy most of the code intact without introducing more bugs to the system.

Advertisements
Why you should avoid using SIGALRM for timer

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s