Re: CVS commit: src/sys/kern

["Kamil Rytarowski" <n54%gmx.com@localhost>]

> Sent: Wednesday, March 29, 2017 at 11:27 PM
> From: "Kamil Rytarowski" <n54%gmx.com@localhost>
> To: "Christos Zoulas" <christos%astron.com@localhost>
> Cc: source-changes-d%NetBSD.org@localhost
> Subject: Re: CVS commit: src/sys/kern
>
> 
> 
> > Sent: Wednesday, March 29, 2017 at 10:09 PM
> > From: "Christos Zoulas" <christos%astron.com@localhost>
> > To: source-changes-d%NetBSD.org@localhost
> > Subject: Re: CVS commit: src/sys/kern
> >
> > In article <20170329195230.D895AFBE5%cvs.NetBSD.org@localhost>,
> > Kamil Rytarowski <source-changes-d%NetBSD.org@localhost> wrote:
> > >-=-=-=-=-=-
> > >
> > >Module Name:	src
> > >Committed By:	kamil
> > >Date:		Wed Mar 29 19:52:30 UTC 2017
> > >
> > >Modified Files:
> > >	src/sys/kern: core_elf32.c sys_ptrace_common.c
> > >
> > >Log Message:
> > >Generate ELF AUXV for core(5) and ptrace(2) limited to the vector TYPE x V
> > >
> > >Previously PT_DUMPCORE and PIOD_READ_AUXV and regular core dumping retrieved
> > >the vector of AuxInfo {a_type, a_v} + MAXPATHLEN + ALIGN(1).
> > >
> > >The extra data is not actually needed in the returned chunk. It can be
> > >retrieved with PT_READ_I operations and it's the preferred way to access
> > >them as the AuxInfo fields contain pointers (void* format) to them.
> > >
> > >This changes the behavior of the kernel, no stable releases are affected
> > >with this move. Current software is not affected as other systems already
> > >stop generating data on AT_NULL. This streamlines the NetBSD behavior with
> > >other ELF format OSes. This move also simplifies determination if we got
> > >all the needed data inside the debugger and we no longer need to eliminate
> > >the unneeded chunk at the end.
> > 
> > This is both incomplete and broken:
> > 
> > 1. The size needed is not just the maximum number of entries * the size of
> >    an entry. You have to account for the string entry storage that
> >    AT_SUN_EXECNAME needs (and alignment).
> 
> My intention was to remove the content of it. Do we need it? Why?
> AT_SUN_EXECNAME has a pointer and we can retrieve it with a debugger.
> I'm not sure it's safe to read and data after trailing AT_NULL and make
> assumptions what is it.
> 
> I checked SunOS (perhaps the best reference for AT_SUN_EXECNAME) and
> they don't offer this either.
> 
> AUXV is stored in /proc/$pid/auxv
> 
> $ uname -rms
> SunOS 5.11 i86pc
> 
> $ hexdump  /home/kamil/auxv.sol.txt                                                                                                                   
> 0000000 07d8 0000 7fcf 0804 07de 0000 7fd5 0804
> 0000010 0019 0000 7fec 0804 0003 0000 0034 0805
> 0000020 0004 0000 0020 0000 0005 0000 0006 0000
> 0000030 0009 0000 6480 0805 07e0 0000 b000 feff
> 0000040 0007 0000 5000 fefb 0008 0000 0000 0000
> 0000050 0006 0000 1000 0000 07e1 0000 0002 0000
> 0000060 07d9 0000 5c6f 7dd7 07e7 0000 0000 0000
> 0000070 0000 0000 0000 0000 0000 0000 0000 0000
> *
> 00000b0
> 
> 07d8 0000 - AT_SUN_PLATFORM
> 07de 0000 - AT_SUN_EXECNAME
> 0019 0000 - AT_RANDOM
> 0003 0000 - AT_PHDR
> 0004 0000 - AT_PHENT
> 0005 0000 - AT_PHNUM
> 0009 0000 - AT_ENTRY
> 07e0 0000 - AT_SUN_LDDATA
> 0007 0000 - AT_BASE
> 0008 0000 - AT_FLAGS
> 0006 0000 - AT_PAGESZ
> 07e1 0000 - AT_SUN_AUXFLAGS
> 07d9 0000 - AT_SUN_HWCAP
> 07e7 0000 - AT_SUN_HWCAP2
> 0000 0000 - AT_NULL
> 
> 
> > 2. There are other places where es_arglen is used, and needs to be cleaned
> >    up not to mention all the size info defines in the emulations
> >    (svr4/osf1/linux).
> 
> I will check emulation core dumps.
> 
> > 3. You can't assume that ELF_AUX_ENTRIES is the max number of entries needed
> >    since emulations might need more (they currently don't) and some need
> >    less.
> > 
> > In other words, you strictly made the situation worse and you possibly broke
> > coredumps. Fortunately you did not break exec code :-)
> > 
> > christos
> > 
> > 
> 

I've checked the emulation code and indeed, there is a variety of possibilities.
There are longer and shorter AUX vectors.

Two simple solutions:
 - revert and keep some trailing content after AT_NULL,
 - check PK_32, stack grow direction and find AT_NULL before dumping the vector.

The former seems safer and I will go for it.

Thanks for pointing it out.