[PATCH] Perl: NULL-terminate argument list

Maxim Dounin mdounin at mdounin.ru
Tue Aug 5 21:26:28 UTC 2014


Hello!

On Tue, Aug 05, 2014 at 03:33:18AM -0700, Piotr Sikora wrote:

> Hey Maxim,
> 
> > Somebody have to investigate these crashes to find out where the
> > problem is.  As previously suggested, nginx code matches perlembed
> > samples, and it looks like as either perl itself or perlembed bug.
> > I also suspect that the patch fixes things due to some
> > side effect, not because it does something needed.
> >
> > If you have no plans to investigate it yourself, you may want to
> > share a way to reproduce the problem.
> 
> I did enough of investigation to be satisfied with the patch I
> provided and I have no plans to dig into perl's code, especially
> because I have no interest in the perl module itself.
> 
> Like I said, this condition can be triggered when using malloc with
> guard pages, when argv[argc] == pool->d.last == pool->d.end.
> 
> For me, it's 100% reproducible when running nginx-tests under OSX's
> guard malloc against fully built nginx built:

[...]

Thanks, I was able to reproduce it here.  It also happily dies 
with the perlembed sample code modified to allocate embedding 
array dynamically.  Looking into further into perl code shows 
this:

Program received signal SIGSEGV, Segmentation fault.
0x000000010002b364 in S_parse_body (env=0x0, xsinit=0x0) at perl.c:2055
2055		scriptname = argv[0];
(gdb) list
2050	#  endif
2051	    }
2052	#endif
2053	
2054	    if (!scriptname)
2055		scriptname = argv[0];
2056	    if (PL_e_script) {
2057		argc++,argv--;
2058		scriptname = BIT_BUCKET;	/* don't look for script or read stdin */
2059	    }
(gdb) p argv[0]
Cannot access memory at address 0x10057b000

I.e., while it generally uses argc to control number of arguments 
used, in some cases it unconditionally tries to use an argument.  
In additional to this place, I see at least one more code path 
where this happens, in "-e" parsing if no parameter follows.

So this indeed looks like perlembed bug, and trailing NULL is 
required due to assumptions in the code.

I've committed the patch, thanks.

-- 
Maxim Dounin
http://nginx.org/



More information about the nginx-devel mailing list