Andy Polyakov wrote:
>> I'm looking for anyone who's using openssl on 386 (and perhaps 486)
>> chip models that might be able to verify a patch before it gets applied?
>
> Recall that if you simply run ./config, the resulting binary code is not
> compatible with 386. You either have to explicitly pass 386 or no-asm
> options. Meaning that I think that it's more than appropriate to #ifndef
> I386_ONLY the whole engine in question, which effectively eliminates the
> need for explicit test on hardware:-)
Done. Can I remove the test for CPUID availability now?
>> Any feedback on this would be most useful, as this feature is only
>> supported on certain x86 chips and so we can't commit unless the
>> detection code is seen to be safe on the remaining x86 chips,
>> particularly older ones without cpuid.
>
> As 0xC000000x used by engine are vendor specific codes, I'd say it's
> essential to identify the CPU vendor prior asking for those codes. Point
> is that another OEM can assign own meaning to it, which could mislead
> us. According to http://www.sandpile.org/ia32/cpuid.htm this can be
> achieved by checking for CentaurHauls ID after CPUID with 0 in %eax.
Done.
> As for the code itself. What's "pushfl; popfl; movl %ebx,-4(%esp)"?
> Especially the last instruction? Is it safe to store something above
> stack pointer? What if a signal is delivered? What's wrong with just
> "push %ebx" and then "pop %ebx"? A.
Sorry, this is my habit from amd64 :-/ I'm not 100% sure about i386 but
on amd64 there is so called "red zone" 128B below %rsp that is preserved
by the signal handler. I.e. it is safe to store something there without
having to worry about sighandlers. But I have nothing against push/pop
and updated the patch.
FYI "pushfl; popfl" is used to clear a bit in eflags that says whether
or not to reload the encryption key from memory. For now I always force
the reloading. Probably it could be more optimized later.
Attached is an interdiff with these changes. The full patch for 0.9.8 at
http://www.logix.cz/michal/devel/padlock/ was also updated.
Any other comments? Or is it ready for inclusion now?
Michal Ludvig
--
* A mouse is a device used to point at the xterm you want to type in.
* Personal homepage - http://www.logix.cz/michal
Index: crypto/engine/eng_padlock.c
===================================================================
--- crypto/engine/eng_padlock.c.orig
+++ crypto/engine/eng_padlock.c
@@ -97,7 +97,7 @@
you must use GNU GCC for now. This is not a "technology
limitation", I simply didn't have a chance to test other
compilers and instead chosen the safe way :-) */
-#if defined(__i386__) && !defined(__amd64__) && defined(__GNUC__)
+#if !defined(I386_ONLY) && defined(__i386__) && !defined(__amd64__) &&
defined(__GNUC__)
#define COMPILE_HW_PADLOCK
#else
#undef COMPILE_HW_PADLOCK
@@ -215,21 +215,30 @@
static int
padlock_available(void)
{
+ char vendor_string[16];
uint32_t eax, edx;
/* First check if the CPUID instruction is available at all... */
if (! padlock_insn_cpuid_available())
return 0;
+ /* Are we running on the Centaur (VIA) CPU? */
+ eax = 0x00000000;
+ vendor_string[12] = 0;
+ asm volatile ("cpuid; movl %%ebx, (%%edi); movl %%edx, 4(%%edi); movl %%ecx,
8(%%edi)"
+ : "+a"(eax) : "mD"(vendor_string) : "ebx", "ecx", "edx");
+ if (strcmp(vendor_string, "CentaurHauls") != 0)
+ return 0;
+
/* Check for Centaur Extended Feature Flags presence */
eax = 0xC0000000;
asm volatile ("pushl %%ebx; cpuid; popl %%ebx" : "+a"(eax) : : "ecx", "edx");
@@ -526,14 +535,14 @@
uint8_t **iv, void *control_word, uint32_t count) \
{ \
asm volatile ("pushfl; popfl\n" \
- "movl %%ebx, -4(%%esp)\n" \
+ "pushl %%ebx\n" \
"movl %%eax, %%ebx\n" \
"movl %0, %%eax\n" \
"movl (%%eax), %%eax\n" \
opcode "\n" \
"movl %0, %%ebx\n" \
"movl %%eax, (%%ebx)\n" \
- "movl -4(%%esp), %%ebx\n" \
+ "popl %%ebx\n" \
: "+m"(iv), "+S"(input), "+D"(output) \
: "a"(key), "c"(count), "d"(control_word)); \
}
