Re: Bug#595403: perl: POSIX::sigaction breakage on armel

[Niko Tyni <ntyni@debian.org>]

reassign 595403 libc6 2.11.2-5
retitle 595403 libc6: sigaction + SA_RESTORER problems on armel
thanks

On Tue, Sep 14, 2010 at 11:20:25PM +0300, Niko Tyni wrote:
> On Fri, Sep 10, 2010 at 02:35:54PM +0300, Niko Tyni wrote:
> > On Fri, Sep 03, 2010 at 09:01:27PM +0300, Niko Tyni wrote:
> > 
> > > > As Sys::SigAction is a pure perl module, I agree the bug is most probably
> > > > in POSIX::sigaction in the perl package.

> Further inspection shows the stack corruption somehow happens between
> the signal delivery and the signal handler invocation. See the gdb
> session below. Returning from the sighandler makes the program crash
> as the return address is smashed.

I've finally got to the bottom of this. It's not stack corruption after
all, it's about the obsolete and non-POSIX SA_RESTORER functionality
in sigaction(2).

The crash in the libsys-sigaction-perl test suite on armel is the result
of three things:

A. sigaction(2) in the C library "leaks" the SA_RESTORER flag
  (0x4000000) into the oldact struct on at least armel and amd64.
  This flag is undocumented, not defined in a public header, and the
  manual page explicitly discourages using the corresponding sa_restorer()
  struct member. See the attached sa-leak.c test.

B. on armel only(?) sigaction(2) in the C library actually honors the
  SA_RESTORER flag: it will pass the sa_restorer() pointer through to
  the kernel syscall when the flag is set. See the attached sa-restorer.c
  test, which crashes on armel.

C. POSIX::sigaction() in the perl package uses a separate Perl side data
  structure for passing the sigaction information in and out. This data
  structure contains fields for the handler, mask and flags, and their
  values are copied to the actual C side sigaction structs. If the flags
  contain the SA_RESTORER bit (0x4000000), it will be passed through
  but the corresponding sa_restorer() member will not be set. See the
  attached sa-perl.c test, which is a C translation of the original problem.

I'm reassigning this to libc6. I think just fixing the leak in the above
item "A" should fix this. I expect the item "B" is a feature and not
a bug.

As for the item "C": I can see perl could work around this by either

- carrying the sa_restorer() pointer over to the Perl side, or
- zeroing out all non-POSIX flag bits on input and/or output

Neither of these seem very inviting or easily justifiable upstream. The
sa_restorer() pointer is explicitly documented not to be used at all,
and zeroing out any unknown bits seems like a risky thing that might
cause breakage on other systems.

Just clearing the SA_RESTORER flag cannot be done portably AFAICS as
it's not defined in a public header file.

@eglibc maintainers: please let me know if I should consider implementing
a workaround anyway; this is blocking the RC bug #595397.
-- 
Niko Tyni   ntyni@debian.org

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

/* not in a public header file */
#ifndef SA_RESTORER
#define SA_RESTORER 0x4000000
#endif

int main(void)
{
    struct sigaction a1, a2;
    sigset_t m;
    
    if (sigemptyset(&m) < 0) {
        perror("sigemptyset");
    }

    a1.sa_handler = SIG_IGN;
    a1.sa_mask    = m;
    a1.sa_flags   = 0x0;

    if (sigaction(SIGALRM, &a1, NULL) < 0) {
        perror("sigaction");
    }
    if (sigaction(SIGALRM, NULL, &a2) < 0) {
        perror("sigaction");
    }
    printf("SA_RESTORER (0x%x) is %s (flags: 0x%x)\n", SA_RESTORER,
      (a2.sa_flags & SA_RESTORER ? "set" : "not set"), a2.sa_flags);
    return 0;
}

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

/* not in a public header file */
#ifndef SA_RESTORER
#define SA_RESTORER 0x4000000
#endif

void h (int s) {
    fprintf(stderr, "caught signal %d\n", s);
}

int main (void) {
    struct sigaction a;
    sigset_t m;

    if (sigemptyset(&m) < 0) {
        perror("sigemptyset");
    }

    memset(&a, 0, sizeof(a));
    a.sa_handler = &h;
    a.sa_mask = m;
    a.sa_flags = SA_RESTORER;

    if (sigaction(SIGALRM, &a, NULL) < 0) {
        perror("sigaction");
    }

    kill(0, SIGALRM);
    printf("survived!\n");
    return(0);
}

#include <stdio.h>
#include <sys/types.h>
#include <signal.h>
#include <unistd.h>
#include <stdlib.h>

struct my_sa_struct {
    void (*handler)(int);
    sigset_t mask;  
    int flags;
};

void h (int s) {
    fprintf(stderr, "caught signal %d\n", s);
}

int my_sigaction(int signum, struct my_sa_struct *new, struct my_sa_struct *old)
{
    int ret;
    struct sigaction act, oldact;

    if (new) {
        act.sa_handler = new->handler;
        act.sa_mask    = new->mask;
        act.sa_flags   = new->flags;
    }
    
    if ((ret = sigaction(signum, &act, &oldact)) < 0) {
        perror("sigaction");
    } 

    if (old) {
        old->handler = oldact.sa_handler;
        old->mask    = oldact.sa_mask;
        old->flags   = oldact.sa_flags;
    }

    return ret;
}

int main (void) {
    struct my_sa_struct old, new;
    sigset_t m;

    if (sigemptyset(&m) < 0) {
        perror("sigemptyset");
    }

    new.handler = &h;
    new.mask = m;
    new.flags = 0;

    my_sigaction(SIGALRM, &new, NULL);
    my_sigaction(SIGALRM, NULL, &old);
    my_sigaction(SIGALRM, &old, NULL);

    kill(0, SIGALRM);
    printf("survived!\n");
    return 0;
}