Hi,
On Wed, Sep 11, 2013 at 8:46 AM, Florian Weimer <[email protected]> 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
--
devel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct
