| Message ID | 20110620135115.GA11874@lucon.org |
|---|---|
| State | New |
| Headers | show |
On 06/20/2011 03:51 PM, H.J. Lu wrote: > Promote pointers to Pmode when passing/returning in registers is > a security concern. Whuh? > * combine.c (cant_combine_insn_p): Check zero/sign extended > hard registers. This part is OK. Bernd
On Mon, Jun 20, 2011 at 6:53 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: > On 06/20/2011 03:51 PM, H.J. Lu wrote: >> Promote pointers to Pmode when passing/returning in registers is >> a security concern. > > Whuh? > Peter, do you think it is safe to assume upper 32bits are zero in user space for x32? Kernel isn't a problem since pointer is 64bit in kernel and we don't pass pointers on stack to kernel.
On 06/20/2011 07:01 AM, H.J. Lu wrote: > On Mon, Jun 20, 2011 at 6:53 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: >> On 06/20/2011 03:51 PM, H.J. Lu wrote: >>> Promote pointers to Pmode when passing/returning in registers is >>> a security concern. No. Promoting *NON*-pointers (or rather, requiring non-pointers to having already been zero extended) is a security concern. I thought I'd made that point clear already. This is a hideously critical distinction. > Peter, do you think it is safe to assume upper 32bits are zero in > user space for x32? Kernel isn't a problem since pointer is 64bit > in kernel and we don't pass pointers on stack to kernel. As I have already stated, if we *cannot* require pointers to be zero-extended on entry to the kernel, we're going to have to have special entry points for all the x32 system calls except the ones that don't take pointers. -hpa
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/20/11 08:33, H. Peter Anvin wrote: > On 06/20/2011 07:01 AM, H.J. Lu wrote: >> On Mon, Jun 20, 2011 at 6:53 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: >>> On 06/20/2011 03:51 PM, H.J. Lu wrote: >>>> Promote pointers to Pmode when passing/returning in registers is >>>> a security concern. > > No. Promoting *NON*-pointers (or rather, requiring non-pointers to > having already been zero extended) is a security concern. I thought I'd > made that point clear already. This is a hideously critical distinction. > >> Peter, do you think it is safe to assume upper 32bits are zero in >> user space for x32? Kernel isn't a problem since pointer is 64bit >> in kernel and we don't pass pointers on stack to kernel. > > As I have already stated, if we *cannot* require pointers to be > zero-extended on entry to the kernel, we're going to have to have > special entry points for all the x32 system calls except the ones that > don't take pointers.asdfasfd BTW (and feel free to respond off-list), what's the rationale behind zero-extending values in x32 from 32 bits to 64 bits rather than the more traditional sign-extending? If it's already been discussed, feel free to point me at the thread. It's not hugely important, but I get this nagging feeling I'm missing something. jeff -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJN/1vpAAoJEBRtltQi2kC7XEUH/0XkfWpqjp3ry/GHsLEYn+8/ bKbd8x5gcrdBKenxkqKrDOqhTGUvttRxN4JClafzWSGFdrDiYxIwJAR4NAvuyYWa rRzSgMRl7VBmPAO+CmEyy2LZ7zEHn4j+iGPWrVQLToAjkIxlSO91Tu/8bimXocC6 FymxVe1Zj9+sCDTmirQnc+TtwnQwVIsNfXWk1e1tKO2bXaUQzfX3YaUKW/B1jw1V 5Xfong0y6XTCDAZQ4sNkCqVYaQBZuMfCLHVX8WP/kA0jMRycvgHWM823YFfFEU+P UHGgrVQeJX5GS1yoCXNrvvBLtDLIbnyYSAjosCIgje7hskBIqKqSdr146q1CTfY= =lUfw -----END PGP SIGNATURE-----
On Mon, Jun 20, 2011 at 7:33 AM, H. Peter Anvin <hpa@zytor.com> wrote: > On 06/20/2011 07:01 AM, H.J. Lu wrote: >> On Mon, Jun 20, 2011 at 6:53 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: >>> On 06/20/2011 03:51 PM, H.J. Lu wrote: >>>> Promote pointers to Pmode when passing/returning in registers is >>>> a security concern. > > No. Promoting *NON*-pointers (or rather, requiring non-pointers to > having already been zero extended) is a security concern. I thought I'd > made that point clear already. This is a hideously critical distinction. I can promote pointers then. >> Peter, do you think it is safe to assume upper 32bits are zero in >> user space for x32? Kernel isn't a problem since pointer is 64bit >> in kernel and we don't pass pointers on stack to kernel. > > As I have already stated, if we *cannot* require pointers to be > zero-extended on entry to the kernel, we're going to have to have > special entry points for all the x32 system calls except the ones that > don't take pointers. > 32bit pointers are zero-extended to 64bit when passing in registers to kernel.
On Mon, Jun 20, 2011 at 7:40 AM, Jeff Law <law@redhat.com> wrote: > >>> Peter, do you think it is safe to assume upper 32bits are zero in >>> user space for x32? Kernel isn't a problem since pointer is 64bit >>> in kernel and we don't pass pointers on stack to kernel. >> >> As I have already stated, if we *cannot* require pointers to be >> zero-extended on entry to the kernel, we're going to have to have >> special entry points for all the x32 system calls except the ones that >> don't take pointers.asdfasfd > BTW (and feel free to respond off-list), what's the rationale behind > zero-extending values in x32 from 32 bits to 64 bits rather than the > more traditional sign-extending? > Since hardware zero-extends 32-bit result to 64-bit in the destination general-purpose register, we can load addresses into 32bit (ptr_mode) register and use the full 64bit (Pmode) register without any additional instructions. As far as the processor is concerned, x32 process is the same as x86-64 process. The only difference is x32 won't go over 4GB.
On 06/20/2011 07:43 AM, H.J. Lu wrote: > On Mon, Jun 20, 2011 at 7:33 AM, H. Peter Anvin <hpa@zytor.com> wrote: >> On 06/20/2011 07:01 AM, H.J. Lu wrote: >>> On Mon, Jun 20, 2011 at 6:53 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: >>>> On 06/20/2011 03:51 PM, H.J. Lu wrote: >>>>> Promote pointers to Pmode when passing/returning in registers is >>>>> a security concern. >> >> No. Promoting *NON*-pointers (or rather, requiring non-pointers to >> having already been zero extended) is a security concern. I thought I'd >> made that point clear already. This is a hideously critical distinction. > > I can promote pointers then. > Yes. The issue comes when promoting non-pointers, like "unsigned int". Pointers are the opposite because pointers in the kernel are 64 bits and we'd like them to be pre-promoted. >>> Peter, do you think it is safe to assume upper 32bits are zero in >>> user space for x32? Kernel isn't a problem since pointer is 64bit >>> in kernel and we don't pass pointers on stack to kernel. >> >> As I have already stated, if we *cannot* require pointers to be >> zero-extended on entry to the kernel, we're going to have to have >> special entry points for all the x32 system calls except the ones that >> don't take pointers. > > 32bit pointers are zero-extended to 64bit when passing in registers to > kernel. Excellent, sounds like we have converged. I saw you posted something about pointers on the stack, but it sounds like you already realized that we don't point any stack arguments whatsoever to the kernel. -hpa
On 06/20/2011 07:33 AM, H. Peter Anvin wrote: > On 06/20/2011 07:01 AM, H.J. Lu wrote: >> On Mon, Jun 20, 2011 at 6:53 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: >>> On 06/20/2011 03:51 PM, H.J. Lu wrote: >>>> Promote pointers to Pmode when passing/returning in registers is >>>> a security concern. > > No. Promoting *NON*-pointers (or rather, requiring non-pointers to > having already been zero extended) is a security concern. I thought I'd > made that point clear already. This is a hideously critical distinction. > >> Peter, do you think it is safe to assume upper 32bits are zero in >> user space for x32? Kernel isn't a problem since pointer is 64bit >> in kernel and we don't pass pointers on stack to kernel. > > As I have already stated, if we *cannot* require pointers to be > zero-extended on entry to the kernel, we're going to have to have > special entry points for all the x32 system calls except the ones that > don't take pointers. If it's a security concern, surely you have to do it in the kernel anyway, lest someone call into the kernel via their own assembly rather than something controlled by the compiler... r~
On 06/20/2011 03:49 PM, Richard Henderson wrote: >> >> As I have already stated, if we *cannot* require pointers to be >> zero-extended on entry to the kernel, we're going to have to have >> special entry points for all the x32 system calls except the ones that >> don't take pointers. > > If it's a security concern, surely you have to do it in the kernel > anyway, lest someone call into the kernel via their own assembly > rather than something controlled by the compiler... > That was the point... right now we rely on the ABI to not have any invalid representations (except, as far as I know, on s390). This means any arbitrary register image presented to the kernel will be a set of valid C objects; we then accept or reject them as being semantically valid using normal C code in the kernel. The issue occurs when the kernel can be entered with something in the register that is invalid according to the calling convention, and not have it rejected. The current x86-64 ABI rules, for example, imply that if %rdi = 0x3fb8c9119537d37d and the type of the first argument is uint32_t, that is a valid argument with the value 0x9537d37d. The extra upper bits are ignored, and so no security issue arises. The issue with requiring the upper bits to be normalized occurs with code like: static const long foo_table[10] = { ... }; long sys_foo(unsigned int bar) { if (bar >= 10) return -EINVAL; return foo_table[bar]; } If the upper bits are required to be zero, gcc could validly translate that to: sys_foo: cmpl $10, %edi jae .L1 movq foo_table(,%rdi,3), %rax retq .L1: movq $-EINVAL, %rax retq Enter this function with a non-normalized %rdi and you have a security hole even though the C is perfectly fine. -hpa
On 06/20/2011 04:39 PM, H. Peter Anvin wrote: > sys_foo: > cmpl $10, %edi > jae .L1 > > movq foo_table(,%rdi,3), %rax > retq > .L1: > movq $-EINVAL, %rax > retq > > Enter this function with a non-normalized %rdi and you have a security > hole even though the C is perfectly fine. Yes, I get that. Isn't it already the case that x86_64 defines the upper half of 32-bit inputs as garbage? Assuming you're never intending to run an x32 kernel, but always an x32 environment within an x86_64 kernel, where does the talk of security holes wrt non-pointers come from? r~
Richard Henderson <rth@redhat.com> wrote: >On 06/20/2011 04:39 PM, H. Peter Anvin wrote: >> sys_foo: >> cmpl $10, %edi >> jae .L1 >> >> movq foo_table(,%rdi,3), %rax >> retq >> .L1: >> movq $-EINVAL, %rax >> retq >> >> Enter this function with a non-normalized %rdi and you have a >security >> hole even though the C is perfectly fine. > >Yes, I get that. Isn't it already the case that x86_64 defines the >upper half of 32-bit inputs as garbage? Assuming you're never >intending >to run an x32 kernel, but always an x32 environment within an x86_64 >kernel, where does the talk of security holes wrt non-pointers come >from? > > >r~ H.J. was proposing an ABI change. I wanted to explain that the current ABI is the way it is for a reason. x32 pointers, however, can and should be zero-extended since they will be 64 bits in the kernel, as you correctly point out.
diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32 index 564e123..6619f2f 100644 --- a/gcc/ChangeLog.x32 +++ b/gcc/ChangeLog.x32 @@ -1,3 +1,16 @@ +2011-06-19 H.J. Lu <hongjiu.lu@intel.com> + + PR middle-end/47725 + PR target/48085 + * calls.c (precompute_register_parameters): Don't convert + pointer to TLS symbol if needed. + + * combine.c (cant_combine_insn_p): Check zero/sign extended + hard registers. + + * config/i386/i386.c (ix86_promote_function_mode): Removed. + (TARGET_PROMOTE_FUNCTION_MODE): Likewise. + 2011-06-15 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 diff --git a/gcc/calls.c b/gcc/calls.c index 5a00d95..3d9a03f 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -706,13 +706,7 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, pseudo now. TLS symbols sometimes need a call to resolve. */ if (CONSTANT_P (args[i].value) && !targetm.legitimate_constant_p (args[i].mode, args[i].value)) - { - if (GET_MODE (args[i].value) != args[i].mode) - args[i].value = convert_to_mode (args[i].mode, - args[i].value, - args[i].unsignedp); - args[i].value = force_reg (args[i].mode, args[i].value); - } + args[i].value = force_reg (args[i].mode, args[i].value); /* If we are to promote the function arg to a wider mode, do it now. */ diff --git a/gcc/combine.c b/gcc/combine.c index d3574a3..5512649 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -2168,6 +2168,12 @@ cant_combine_insn_p (rtx insn) return 0; src = SET_SRC (set); dest = SET_DEST (set); + if (GET_CODE (src) == ZERO_EXTEND + || GET_CODE (src) == SIGN_EXTEND) + src = XEXP (src, 0); + if (GET_CODE (dest) == ZERO_EXTEND + || GET_CODE (dest) == SIGN_EXTEND) + dest = XEXP (dest, 0); if (GET_CODE (src) == SUBREG) src = SUBREG_REG (src); if (GET_CODE (dest) == SUBREG) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index fa5ae97..104767b 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -7050,23 +7050,6 @@ ix86_function_value (const_tree valtype, const_tree fntype_or_decl, return ix86_function_value_1 (valtype, fntype_or_decl, orig_mode, mode); } -/* Pointer function arguments and return values are promoted to - Pmode. */ - -static enum machine_mode -ix86_promote_function_mode (const_tree type, enum machine_mode mode, - int *punsignedp, const_tree fntype, - int for_return) -{ - if (for_return != 1 && type != NULL_TREE && POINTER_TYPE_P (type)) - { - *punsignedp = POINTERS_EXTEND_UNSIGNED; - return Pmode; - } - return default_promote_function_mode (type, mode, punsignedp, fntype, - for_return); -} - rtx ix86_libcall_value (enum machine_mode mode) { @@ -35132,9 +35115,6 @@ ix86_autovectorize_vector_sizes (void) #undef TARGET_FUNCTION_VALUE_REGNO_P #define TARGET_FUNCTION_VALUE_REGNO_P ix86_function_value_regno_p -#undef TARGET_PROMOTE_FUNCTION_MODE -#define TARGET_PROMOTE_FUNCTION_MODE ix86_promote_function_mode - #undef TARGET_SECONDARY_RELOAD #define TARGET_SECONDARY_RELOAD ix86_secondary_reload
Promote pointers to Pmode when passing/returning in registers is a security concern. This patch removes ix86_promote_function_mode, which exposes: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47725 There are 2 different patches for PR 47725: http://gcc.gnu.org/ml/gcc-patches/2011-02/threads.html#01018 One checks zero/sign extended hard registers in cant_combine_insn_p and the other changes assign_parm_setup_reg to copy the hard register first before extending it. This patch changes cant_combine_insn_p. H.J. ---- commit 6202f3601f5b2a5a41b60425f1206681823ecaa9 Author: H.J. Lu <hjl.tools@gmail.com> Date: Sun Jun 19 19:24:11 2011 -0700 Remove ix86_promote_function_mode. 2011-06-19 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/47725 PR target/48085 * calls.c (precompute_register_parameters): Don't convert pointer to TLS symbol if needed. * combine.c (cant_combine_insn_p): Check zero/sign extended hard registers. * config/i386/i386.c (ix86_promote_function_mode): Removed. (TARGET_PROMOTE_FUNCTION_MODE): Likewise.