I mentioned this bug previously. Here's the basic explanation of what's
wrong with the current code in fwcutter.c:
1. Consider a little-endian binary containing ucode firmware. Each
8-byte instruction is stored with byte order 01234567 (with 0
being the least significant byte.)
2. In extract_or_identify(), the byte order is
reversed /32-bit-wise/ since the b43 driver expects big-endian
32-bit integers. After this transformation, the instructions
are stored with byte order 32107654.
3. In disasm_ucode_1(), disasm_ucode_2(), disasm_ucode_3(), the
upper and lower halves of the instruction are swapped. Now the
instruction's byte order is 76543210.
4. These routines treat the instruction as a *NATIVE* 64-bit
integer, and break it down into bitfields. The byte order is
correct on big-endian hosts, but completely reversed for
little-endians hosts.
The source is quite confusing because it manipulates the ucode as 4-byte
quantities for purposes of output endianness, but then tries to analyze
them as 8-byte instructions. This is presumably the source of the bug.
My patch avoids reordering the ucode endianness until after running
analyse_ucode() to extract version information. It passes the firmware
flags to analyse_ucode() so that it can correctly convert each
instruction to a native-endian 64-bit integer.
TO TEST THIS PATCH: try extracting firmware FW13 on a little-endian
host. The unpatched b43-fwcutter will not identify ucode
version/revision/date/time, but with this patch it will.
Dan
--- fwcutter.c.orig 2009-06-02 11:52:55.000000000 -0400
+++ fwcutter.c 2009-09-15 14:26:49.000000000 -0400
@@ -42,6 +42,7 @@
#include <sys/endian.h>
#else
#include <byteswap.h>
+#include <endian.h>
#endif
#include "md5.h"
@@ -143,24 +144,19 @@
printf(" ucode revision: %d\n", val);
break;
case 2:
- printf(" ucode date: %.4d-%.2d-%.2d\n",
+ printf(" ucode date: %.4d-%.2d-%.2d\n",
2000 + (val >> 12), (val >> 8) & 0xF, val & 0xFF);
break;
case 3:
- printf(" ucode time: %.2d:%.2d:%.2d\n",
- val >> 11, (val >> 5) & 0x3f, val & 0x1f);
+ printf(" ucode time: %.2d:%.2d:%.2d\n",
+ val >> 11, (val >> 5) & 0x3f, (val & 0x1f) << 1);
break;
}
}
static void disasm_ucode_1(uint64_t in, struct insn *out)
{
- /* xxyyyzzz00oooooX -> ooooo Xxx yyy zzz
- * if we swap the upper and lower 32-bits first it becomes easier:
- * 00oooooxxxyyyzzz -> ooooo xxx yyy zzz
- */
- in = (in >> 32) | (in << 32);
-
+ /* 00oooooxxxyyyzzz -> ooooo xxx yyy zzz */
out->op3 = in & 0xFFF;
out->op2 = (in >> 12) & 0xFFF;
out->op1 = (in >> 24) & 0xFFF;
@@ -170,12 +166,7 @@
static void disasm_ucode_2(uint64_t in, struct insn *out)
{
- /* xxyyyzzz0000oooX -> ooo Xxx yyy zzz
- * if we swap the upper and lower 32-bits first it becomes easier:
- * 0000oooxxxyyyzzz -> ooo xxx yyy zzz
- */
- in = (in >> 32) | (in << 32);
-
+ /* 0000oooxxxyyyzzz -> ooo xxx yyy zzz */
out->op3 = in & 0xFFF;
out->op2 = (in >> 12) & 0xFFF;
out->op1 = (in >> 24) & 0xFFF;
@@ -189,8 +180,6 @@
* like 2, but each operand has one bit more; appears
* to use the same instruction set slightly extended
*/
- in = (in >> 32) | (in << 32);
-
out->op3 = in & 0x1FFF;
out->op2 = (in >> 13) & 0x1FFF;
out->op1 = (in >> 26) & 0x1FFF;
@@ -198,24 +187,28 @@
/* the rest of the in word should be zero */
}
-static void analyse_ucode(int ucode_rev, uint8_t *data, uint32_t len)
+static void analyse_ucode(int ucode_rev, uint8_t *data, uint32_t len, uint32_t flags)
{
uint64_t *insns = (uint64_t*)data;
+ uint64_t inscode;
struct insn insn;
uint32_t i;
for (i=0; i<len/sizeof(*insns); i++) {
+ if (flags & FW_FLAG_LE) inscode = le64toh(insns[i]);
+ else inscode = be64toh(insns[i]);
+
switch (ucode_rev) {
case 1:
- disasm_ucode_1(insns[i], &insn);
+ disasm_ucode_1(inscode, &insn);
print_ucode_version(&insn);
break;
case 2:
- disasm_ucode_2(insns[i], &insn);
+ disasm_ucode_2(inscode, &insn);
print_ucode_version(&insn);
break;
case 3:
- disasm_ucode_3(insns[i], &insn);
+ disasm_ucode_3(inscode, &insn);
print_ucode_version(&insn);
break;
}
@@ -376,9 +369,18 @@
case EXT_UCODE_1:
ucode_rev += 1;
data_length = extract->length;
+ /* AFAICT, the instructions are stored as uint64 in
+ * the standard endianness for the target. So
+ * analyse_ucode() needs to know their endianness.
+ */
+ analyse_ucode(ucode_rev, buf, data_length, flags);
+
+ /* ucode firmware files require buf to be stored as a
+ * series of big-endian 32-bit integers, so we convert
+ * them if needed before outputting them.
+ */
if (flags & FW_FLAG_LE)
swap_endianness_ucode(buf, data_length);
- analyse_ucode(ucode_rev, buf, data_length);
hdr.type = FW_TYPE_UCODE;
hdr.size = to_be32(data_length);
break;
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
