when building chromium with debug options the resulting chrome binary is huge: ozzie Release # ls -alh chrome -rwxr-xr-x 1 portage portage 2.3G Aug 18 12:55 chrome* As such, paxctl fails to mark it: ozzie Release # paxctl -m chrome file chrome is too big this leads to broken chrome. Interestingly, both paxctl-ng and scanelf can mark it just fine, however the eclass is written so that if paxctl is found it is used, and if it fails, then just fail and don't try anything else. It might be best to just cascade and try one after the other instead of only trying to mark with the first capable program found.
appropriate snippit of build.log: * PT PaX marking -m with paxctl * out/Release/chrome * Failed to set PT_PAX markings -m for: * out/Release/chrome
The failing code is in paxctl.c where if (st.st_size < 0 || INT_MAX < st.st_size) { close(fd); if (!state->quiet) fprintf(stderr, "file %s is too big\n", state->argv[state->files]); return EXIT_FAILURE; } On amd64, INT_MAX is 4G unsigned but st_size from struct stat is of type off_t which is 64 bits wide, so the check is wrong.
We probably want to make the pax-utils.eclass more robust too. Right now it written with the assumption that one method will be used for *all* the files or else its a failure. A better approach is to try each method on a file by file basis. So we should change the eclass to be someething like if has PT ${PAX_MARKINGS}; then _pax_list_files einfo "$@" for f in "$@"; do #First try paxctl -> this might try to create/convert program headers if type -p paxctl > /dev/null; then einfo "PT PaX marking -${flags} ${f} with paxctl" # First, try modifying the existing PAX_FLAGS header paxctl -q${flags} "${f}" && continue # Second, try creating a PT_PAX header (works on ET_EXEC) # Even though this is less safe, most exes need it, eg bug #463170 paxctl -qC${flags} "${f}" && continue # Third, try stealing the (unused under PaX) PT_GNU_STACK header paxctl -qc${flags} "${f}" && continue #Next try paxctl-ng -> this will not create/convert any program headers elif type -p paxctl-ng > /dev/null && paxctl-ng -L ; then einfo "PT PaX marking -${flags} ${f} with paxctl-ng" flags="${flags//z}" [[ ${dodefault} == "yes" ]] && paxctl-ng -L -z "${f}" [[ "${flags}" ]] || continue paxctl-ng -L -${flags} "${f}" && continue #Finally fall back on scanelf elif type -p scanelf > /dev/null && [[ ${PAX_MARKINGS} != "none" ]]; then einfo "Fallback PaX marking -${flags} with scanelf" scanelf -Xxz ${flags} "$f" #We failed to set PT_PAX flags elif [[ ${PAX_MARKINGS} != "none" ]]; then elog "Failed to set PT_PAX markings -${flags} ${f} for:" _pax_list_files elog ${f} ret=1 fi done fi and similarly if has XT ${PAX_MARKINGS}.
Created attachment 383086 [details] New proposed pax-utils.eclass.
Created attachment 383088 [details] Fix indentation
(In reply to Anthony Basile from comment #5) > Created attachment 383088 [details] > Fix indentation This doesn't fix the original issue because the conditionals are wrong: if type -p paxctl > /dev/null; then ... elif type -p paxctl-ng > /dev/null && paxctl-ng -L ; then This means that if paxctl is found, it won't hit the "else if" for paxctl-ng at all. It should be if not elif.
(In reply to Rick Farina (Zero_Chaos) from comment #6) > (In reply to Anthony Basile from comment #5) > > Created attachment 383088 [details] > > Fix indentation > > This doesn't fix the original issue because the conditionals are wrong: > > if type -p paxctl > /dev/null; then > ... > elif type -p paxctl-ng > /dev/null && paxctl-ng -L ; then > > > This means that if paxctl is found, it won't hit the "else if" for paxctl-ng > at all. It should be if not elif. Right. Given the continue's changing the elif's to just ifs should fix that. Let's keep reviewing before I upload the next version, in case there are more issues I'm missing.
(In reply to Anthony Basile from comment #7) > Right. Given the continue's changing the elif's to just ifs should fix > that. Let's keep reviewing before I upload the next version, in case there > are more issues I'm missing. That was everything I saw, but more eyes certainly won't hurt. C'mon hardened team et al, weigh in.
Created attachment 383452 [details] Correct logic wrt to failures @zero_chaos, please test if this works in you above failure with chrome and if it does, I'll email gentoo-dev@ for a final review before committing.
* out/Release/chrome * PT PaX marking -m out/Release/chrome with paxctl * PT PaX marking -m out/Release/chrome with paxctl-ng ozzie eclass # paxctl -v /usr/lib64/chromium-browser/chrome PaX control v0.7 Copyright 2004,2005,2006,2007,2009,2010,2011,2012 PaX Team <pageexec@freemail.hu> - PaX flags: -----m-x-e-- [/usr/lib64/chromium-browser/chrome] MPROTECT is disabled RANDEXEC is disabled EMUTRAMP is disabled looks like this works great to me.
(In reply to Anthony Basile from comment #2) > The failing code is in paxctl.c where > > if (st.st_size < 0 || INT_MAX < st.st_size) { > close(fd); > if (!state->quiet) > fprintf(stderr, "file %s is too big\n", state->argv[state->files]); > return EXIT_FAILURE; > } > > On amd64, INT_MAX is 4G unsigned but st_size from struct stat is of type > off_t which is 64 bits wide, so the check is wrong. INT_MAX is 2G(-1) everywhere in the gcc world since 'int' is a signed 32 bit type. the idea behind the check is to prevent the truncation on 32 bit archs when casting off_t to size_t later but i guess the limit should be SIZE_MAX instead of INT_MAX (at least when -D_FILE_OFFSET_BITS=64 but iirc _GNU_SOURCE enables that).
(In reply to Rick Farina (Zero_Chaos) from comment #10) > looks like this works great to me. Okay its on the tree with a few minor changes discussed on the list. Please keep testing.
(In reply to PaX Team from comment #11) > (In reply to Anthony Basile from comment #2) > > The failing code is in paxctl.c where > > > > if (st.st_size < 0 || INT_MAX < st.st_size) { > > close(fd); > > if (!state->quiet) > > fprintf(stderr, "file %s is too big\n", state->argv[state->files]); > > return EXIT_FAILURE; > > } > > > > On amd64, INT_MAX is 4G unsigned but st_size from struct stat is of type > > off_t which is 64 bits wide, so the check is wrong. > > INT_MAX is 2G(-1) everywhere in the gcc world since 'int' is a signed 32 bit > type. the idea behind the check is to prevent the truncation on 32 bit archs > when casting off_t to size_t later but i guess the limit should be SIZE_MAX > instead of INT_MAX (at least when -D_FILE_OFFSET_BITS=64 but iirc > _GNU_SOURCE enables that). 1) Ack on INT_MAX. 2) SIZE_MAX is simply defined in stdint.h as /* Limit of `size_t' type. */ # if __WORDSIZE == 64 # define SIZE_MAX (18446744073709551615UL) # else # define SIZE_MAX (4294967295U) # endif is this okay when on a 32-bit with -D_FILE_OFFSET_BITS=64 ?
(In reply to Anthony Basile from comment #13) > is this okay when on a 32-bit with -D_FILE_OFFSET_BITS=64 ? i settled on LONG_MAX instead to avoid casting between signed/unsigned types and i guess noone will care about large file support on 32 bit archs anyway. can you give this patch a try? --- a/paxctl.c +++ b/paxctl.c @@ -1,10 +1,11 @@ */ #define _GNU_SOURCE +#define _FILE_OFFSET_BITS 64 #include <stdio.h> #include <stdlib.h> #include <unistd.h> @@ -121,7 +122,7 @@ static int pax_verify_file(struct pax_state * const state) return EXIT_FAILURE; } - if (st.st_size < 0 || INT_MAX < st.st_size) { + if (st.st_size < 0 || LONG_MAX < st.st_size) { close(fd); if (!state->quiet) fprintf(stderr, "file %s is too big\n", state->argv[state->files]);
Created attachment 384118 [details, diff] big file patch for paxctl Please note, attaching patches to the bug is vastly preferred to inlining them, I have a below 0% success rate with inline patches. That said, I manually applied it and it seems to work just fine. Thanks.
(In reply to Rick Farina (Zero_Chaos) from comment #15) > Please note, attaching patches to the bug is vastly preferred to inlining > them, I have a below 0% success rate with inline patches. i just wanted someone to try it quickly (it's a simple 2-liner after all), i'll of course put it into a real paxctl release next, thanks for testing it!
paxctl 0.9 is out and should fix this problem on 64 bit archs.
Some fairly non-trivial eclass improvements and a new release of paxctl with a bug fixed. If only all bugs could be ended this quickly and properly. Great work to all, and thanks.