The argc validation seems broken: $ grep -rw NCARGS /usr/include /usr/include/sys/param.h:#define NCARGS ARG_MAX /* max bytes for an exec function */ $ egrep "argc < 0.*argc" src/lib/libc/*/gen/makecontext.c src/lib/libc/amd64/gen/makecontext.c: else if ((argc < 0) || (argc > 6) || (ucp->uc_stack.ss_sp == NULL) || src/lib/libc/i386/gen/makecontext.c: else if ((argc < 0) || (argc > NCARGS)) { src/lib/libc/ia64/gen/makecontext.c: if (argc < 0 || argc > 8 || ucp == NULL || src/lib/libc/mips/gen/makecontext.c: if (argc < 0 || argc > 6 || ucp == NULL || src/lib/libc/powerpc/gen/makecontext.c: if ((ucp == NULL) || (argc < 0) || (argc > NCARGS) src/lib/libc/powerpc64/gen/makecontext.c: if ((ucp == NULL) || (argc < 0) || (argc > NCARGS) src/lib/libc/sparc64/gen/makecontext.c: if ((argc < 0) || (argc > 6) || $
On Mon, 19 May 2014, Peter Holm wrote: >> Description: > The argc validation seems broken: Why should it do any validation at all? Its man page documents (much more clearly than most man pages) that the behaviour for invalid args is undefined. It can't possibly do any useful validation, since ucp points to mostly-opaque data. All it can do is check that ucp is not a null pointer and that a bit of data pointed to it is reasonable, and that argc is reasonable. It also cannot check that the extra args given by argc are reasonable. > $ grep -rw NCARGS /usr/include > /usr/include/sys/param.h:#define NCARGS ARG_MAX /* max bytes for an exec function */ Bogus. makecontext() isn't an exec function. A random integer would work almost as well here. An arbitrary limit gratuitously restricts makecontext(). Negative and small positive values for the limit would just restrict it enough to break it. It mallocs storage related to argc. A large argc can cause the size calculations for the malloc() to fail, but that is just a special case of undefined behaviour from invalid args. The behaviour is undefined unless argc matches the actual number of args. argc is just a helper for the variadic args. The limit on the number of args for makecontext() is equally poorly documented to the limit on the number of args for a C function. The C standard requires some minimum limit that is too small, and IIRC it requires the actual limit to be documented, but documentation about this is hard to find. In practice, the limit is unreachable except on 16-bit systems (and when someone uses ulimit(1) to demonstrate brokenness). > $ egrep "argc < 0.*argc" src/lib/libc/*/gen/makecontext.c > src/lib/libc/amd64/gen/makecontext.c: else if ((argc < 0) || (argc > 6) || (ucp->uc_stack.ss_sp == NULL) || 6 is an even more gratuitously restrictive limit. > src/lib/libc/i386/gen/makecontext.c: else if ((argc < 0) || (argc > NCARGS)) { > src/lib/libc/ia64/gen/makecontext.c: if (argc < 0 || argc > 8 || ucp == NULL || > src/lib/libc/mips/gen/makecontext.c: if (argc < 0 || argc > 6 || ucp == NULL || > src/lib/libc/powerpc/gen/makecontext.c: if ((ucp == NULL) || (argc < 0) || (argc > NCARGS) > src/lib/libc/powerpc64/gen/makecontext.c: if ((ucp == NULL) || (argc < 0) || (argc > NCARGS) > src/lib/libc/sparc64/gen/makecontext.c: if ((argc < 0) || (argc > 6) || The only bugs I see here are: - the existence of these checks - the style bugs in them: - excessive parentheses for amd64, i386, powerpc and sparc64 - other gratuitous differences, like having the ucp == NULL check first for powerpc and elsewhere for amd64, i386 and sparc64. The complete file differences are even larger due to different birthplaces and not much convergence. Bruce
For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped