numatop: %{optflags} fail the 32bit build

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Thu Sep 12 12:11:02 UTC 2013


Hi,

On Wed, Sep 11, 2013 at 8:46 AM, Florian Weimer <fweimer at redhat.com> wrote:
> On 09/11/2013 12:31 AM, Dridi Boukelmoune wrote:
>
>> I have my first packaging issue on the numatop package[1].
>>
>> During the review it appeared that I forgot the %{optflags}, and that
>> adding them breaks the i686 build. The upstream dev is very patient
>> and willing to help me, but I feel I have wasted enough of his time.
>>
>> The guilty gcc flag seems to be:
>> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 [2]
>
>
> It would be helpful to provide more context and pointers to the analysis so
> far.
>
> The failing build log is here:
>
> http://kojipkgs.fedoraproject.org//work/tasks/1414/5911414/build.log
>
> This is the offending function:
>
> void
> cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
>       unsigned int *edx)
> {
>   __asm volatile
>     ("cpuid\n\t"
>      : "=a" (*eax),
>        "=b" (*ebx),
>        "=c" (*ecx),
>        "=d" (*edx)
>      : "a" (*eax));
> }
>
> The cryptic GCC error message (inconsistent operand constraints in an ‘asm’)
> refers to the fact that in PIC mode (which is activated by the hardening
> flags), %ebx is a reserved register.

Thanks for the explanation, I could never have figured this out.

> This version should work in 32 bit mode, and only in 32 bit mode:
>
> void
> cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
>       unsigned int *edx)
> {
>   __asm volatile
>     ("push %%ebx\n\t"
>      "cpuid\n\t"
>      "mov %%ebx, (%1)\n\t"
>      "pop %%ebx"
>      : "=a" (*eax),
>        "=S" (ebx),
>        "=c" (*ecx),
>        "=d" (*edx)
>      : "0" (*eax));
> }

I "kind of" understand what you're doing here, but it's not all clear.

I get the push/pop instructions save and restore the reserved ebx
register, which is needed because apparently the cpuid instruction
would otherwise overwrite it.

I don't understand the mov instruction, but I suppose you're storing
ebx's value from cpuid "somewhere else" before restoring it with the
pop instruction.

I don't understand the last 5 lines of __asm in both functions, I've
never seen this syntax before.

> I have not actually tested this.  There are other solutions floating around,
> but they are clearly incorrect and produce wrong code.

It builds, but it segfaults. Probably because the value is written
directly in the ebx variable, which is a pointer. I've added a temp
variable to fix the segfault, but I still need to check whether it
gives the expected value:

void
cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
      unsigned int *edx)
{
#ifdef __x86_64
// original version
#else
  unsigned int tmp;
  __asm volatile
    ("push %%ebx\n\t"
     "cpuid\n\t"
     "mov %%ebx, (%1)\n\t"
     "pop %%ebx"
     : "=a" (*eax),
       "=S" (tmp),
       "=c" (*ecx),
       "=d" (*edx)
     : "0" (*eax));
  *ebx = tmp;
#endif
}

I don't actually have the code on this computer but I remember doing
something like that, so this is only pseudo code :)

> In 64 bit mode, you should use the original version.
>
> --
> Florian Weimer / Red Hat Product Security Team

Thank you both for your help, I'll find some time this weekend to test
it further before sending it back to the upstream project.

Dridi


More information about the devel mailing list