[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#875927: perl: SIGUNUSED removal in glibc 2.26 changes PL_sig_name / SIG_SIZE



On Sat, Sep 16, 2017 at 10:27:26AM +0300, Niko Tyni wrote:
> Package: perl
> Version: 5.26.0-8
> Severity: normal
> X-Debbugs-Cc: doko@debian.org, vorlon@debian.org, glibc@packages.debian.org
> 
> As seen in
>  https://bugs.launchpad.net/ubuntu/+source/libanyevent-perl/+bug/1717367
> an upstream change in glibc 2.26, presumably
> https://patchwork.sourceware.org/patch/20800/ causes perl to change its
> ABI when rebuilt against the new glibc. I'm copying Steve and Matthias
> who worked on the issue on the Ubuntu side.
> 
> The problem is that Perl's global array PL_sig_name[] and the SIG_SIZE
> constant ($Config{sig_name}, $Config{sig_size} on the perl language side)
> change due to the removal of the (long?) obsolete SIGUNUSED constant in
> glibc. The Async::Interrupt (libasync-intterupt-perl) XS module compiles
> in SIG_SIZE and walks PL_sig_name[] at run time based on that.  When the
> array changes on the Perl side, Async::Interrupt does not know that and
> accesses it out of bounds.

> Looking at codesearch.debian.net, this also concerns (reverse dependencies
> of) libio-aio-perl, libcoro-perl and libev-perl.

Perl upstream is not going to do anything about this. Their recommendation
is to hardcode the list of signals when calling Configure. This
seems somewhat over the top to me. It might be nicer to temporarily
patch Configure (or from 5.26.1-1 onward rather its sources, mainly
regen-configure/dist/U/Signal.U) to add SIGUNUSED to the list of signals
if it's missing.

Alternatively, postprocessing config.sh should work too and is arguably
cleaner. Ubuntu is currently postprocessing config.h which I'm not
quite as comfortable about as I think it leaves the relevant %Config
entries unchanged.

Regardless of the approach taken, I note that the stashed Configure
results in debian/cross/*/config.sh.static show that SIGUNUSED does not
exist on all architectures, so adding it unconditionally would be wrong.
These ones don't seem to have it:

  % for f in debian/cross/*/config.sh.static; do grep -q '^sig_name_init.*UNUSED' $f || echo $f; done
  debian/cross/alpha/config.sh.static
  debian/cross/hurd-i386/config.sh.static
  debian/cross/kfreebsd-amd64/config.sh.static
  debian/cross/kfreebsd-i386/config.sh.static
  debian/cross/mips64el/config.sh.static
  debian/cross/mips/config.sh.static
  debian/cross/mipsel/config.sh.static
  debian/cross/sparc64/config.sh.static
 
AFAICS this means we need a hardcoded white- or blacklist of the affected
architectures somewhere (and renders the issue even more unactionable
upstream.)

Upstream also suggests that the modules breaking due to this change
are unnecessarily fragile. They all seem to be using the same code,
namely an 's_signum()' function in schmorp.h. Quoting Leon Timmermans:

> The patch to the modules would be little more than removing code;
> they'd just have to use the whichsig (perl 1.0) /whichsig_sv (perl 5.16)
> API instead of reimplementing it. Alternatively s_signame could check
> for the end of PL_sig_name by checking for a null-pointer instead of
> assuming it has SIG_SIZE elements. Unlike the other solutions that will
> prevent this kind of situation from happening again.

Tony Cook also notes that these s_signum() implementations are buggy in
any case, as shown with

 perl -MAsync::Interrupt -le 'print Async::Interrupt::sig2num("POLL")' 

which should give 29 instead of 67 on amd64. The code should look the
signal number up in PL_sig_num[] instead of assuming the signal number
is same as the array index.

I'm tempted to just declare that anything relying on these parts of the
ABI is buggy and get the known affected modules/packages fixed.
-- 
Niko Tyni   ntyni@debian.org


Reply to: