Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Rafał Miłecki]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

I use attached patch to fake result of read[bwl] performed by closed
source driver (ndiswrapper+bcmwl and wl).

1) It works great on my Sony VAIO with Intel(R) Core(TM)2 Duo CPU P8400
2) It locks up Macbook Pro 8,1 with some 8-cores Intel

Do you have any idea why it causes the lockup? Function causing
problem is "set_ins_reg_val". I've created it as copy of
get_ins_reg_val, it just sets values in struct pt_regs, instead of
reading them).

-- 
Rafał

[-- Attachment #2: mmio.debugging.patch --]
[-- Type: application/octet-stream, Size: 3947 bytes --]

diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c
index 3adff7d..f8513bb 100644
--- a/arch/x86/mm/mmio-mod.c
+++ b/arch/x86/mm/mmio-mod.c
@@ -206,6 +206,12 @@ static void pre(struct kmmio_probe *p, struct pt_regs *regs,
 	put_cpu_var(pf_reason);
 }
 
+#define MMIO_BASE			0xfaafc000
+#define B43_MMIO_PHY_CONTROL		0x3FC
+#define B43_MMIO_PHY_DATA		0x3FE
+
+static u16 broadcom_phy_addr;
+
 static void post(struct kmmio_probe *p, unsigned long condition,
 							struct pt_regs *regs)
 {
@@ -219,6 +225,18 @@ static void post(struct kmmio_probe *p, unsigned long condition,
 		BUG();
 	}
 
+	if (my_reason->type == REG_READ && my_trace->phys == MMIO_BASE + B43_MMIO_PHY_DATA) {
+		pr_info("ZAJEC: read PHY 0x%X\n", broadcom_phy_addr);
+		switch (broadcom_phy_addr){
+		case 0x20:
+		case 0x22:
+		case 0x27:
+			pr_info("ZAJEC: overwriting 0x%X with 0xFFFF\n", broadcom_phy_addr);
+			set_ins_reg_val(my_reason->ip, regs, 0xFFFF);
+			break;
+		}
+	}
+
 	switch (my_reason->type) {
 	case REG_READ:
 		my_trace->value = get_ins_reg_val(my_reason->ip, regs);
@@ -227,6 +245,11 @@ static void post(struct kmmio_probe *p, unsigned long condition,
 		break;
 	}
 
+	if (my_reason->type == REG_WRITE && my_trace->phys == MMIO_BASE + B43_MMIO_PHY_CONTROL) {
+		broadcom_phy_addr = my_trace->value;
+		//pr_info("ZAJEC: setting PHY addr to 0x%X\n", broadcom_phy_addr);
+	}
+
 	mmio_trace_rw(my_trace);
 	put_cpu_var(cpu_trace);
 	put_cpu_var(pf_reason);
diff --git a/arch/x86/mm/pf_in.c b/arch/x86/mm/pf_in.c
index 9f0614d..86449ed 100644
--- a/arch/x86/mm/pf_in.c
+++ b/arch/x86/mm/pf_in.c
@@ -461,6 +461,99 @@ err:
 	return 0;
 }
 
+static void set_reg_w32(int no, struct pt_regs *regs, u32 val)
+{
+	switch (no) {
+	case arg_AX:
+		regs->ax = val;
+		break;
+	case arg_BX:
+		regs->bx = val;
+		break;
+	case arg_CX:
+		regs->cx = val;
+		break;
+	case arg_DX:
+		regs->dx = val;
+		break;
+	case arg_SP:
+		regs->sp = val;
+		break;
+	case arg_BP:
+		regs->bp = val;
+		break;
+	case arg_SI:
+		regs->si = val;
+		break;
+	case arg_DI:
+		regs->di = val;
+		break;
+#ifdef __amd64__
+	case arg_R8:
+		regs->r8 = val;
+		break;
+	case arg_R9:
+		regs->r9 = val;
+		break;
+	case arg_R10:
+		regs->r10 = val;
+		break;
+	case arg_R11:
+		regs->r11 = val;
+		break;
+	case arg_R12:
+		regs->r12 = val;
+		break;
+	case arg_R13:
+		regs->r13 = val;
+		break;
+	case arg_R14:
+		regs->r14 = val;
+		break;
+	case arg_R15:
+		regs->r15 = val;
+		break;
+#endif
+	default:
+		printk(KERN_ERR "mmiotrace: Error reg no# %d\n", no);
+	}
+}
+
+void set_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs, u32 val)
+{
+	unsigned int opcode;
+	int reg;
+	unsigned char *p;
+	struct prefix_bits prf;
+	int i;
+
+	p = (unsigned char *)ins_addr;
+	p += skip_prefix(p, &prf);
+	p += get_opcode(p, &opcode);
+	for (i = 0; i < ARRAY_SIZE(reg_rop); i++)
+		if (reg_rop[i] == opcode)
+			goto do_work;
+
+	for (i = 0; i < ARRAY_SIZE(reg_wop); i++)
+		if (reg_wop[i] == opcode)
+			goto do_work;
+
+	printk(KERN_ERR "mmiotrace: Not a register instruction, opcode "
+							"0x%02x\n", opcode);
+	return;
+
+do_work:
+	/* for STOS, source register is fixed */
+	if (opcode == 0xAA || opcode == 0xAB) {
+		reg = arg_AX;
+	} else {
+		unsigned char mod_rm = *p;
+		reg = ((mod_rm >> 3) & 0x7) | (prf.rexr << 3);
+	}
+
+	set_reg_w32(reg, regs, val);
+}
+
 unsigned long get_ins_imm_val(unsigned long ins_addr)
 {
 	unsigned int opcode;
diff --git a/arch/x86/mm/pf_in.h b/arch/x86/mm/pf_in.h
index e05341a..90b43ff 100644
--- a/arch/x86/mm/pf_in.h
+++ b/arch/x86/mm/pf_in.h
@@ -34,6 +34,7 @@ enum reason_type {
 enum reason_type get_ins_type(unsigned long ins_addr);
 unsigned int get_ins_mem_width(unsigned long ins_addr);
 unsigned long get_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs);
+void set_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs, u32 val);
 unsigned long get_ins_imm_val(unsigned long ins_addr);
 
 #endif /* __PF_H_ */

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Pekka Paalanen]

On Sat, 18 Jun 2011 00:31:32 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> I use attached patch to fake result of read[bwl] performed by
> closed source driver (ndiswrapper+bcmwl and wl).
> 
> 1) It works great on my Sony VAIO with Intel(R) Core(TM)2 Duo CPU
> P8400 2) It locks up Macbook Pro 8,1 with some 8-cores Intel
> 
> Do you have any idea why it causes the lockup? Function causing
> problem is "set_ins_reg_val". I've created it as copy of
> get_ins_reg_val, it just sets values in struct pt_regs, instead of
> reading them).

Sorry, I have no insight to that... does unmodified mmiotrace
work properly? Are you tracing the exact same kernel binary blob
on both machines? Maybe it's using some rare instruction
mmiotrace does not decode properly? Maybe with a rep prefix?
Do those CPUs have any differences in their registers or
struct pt_regs?

I'm not even sure how "legal" it is to poke pt_regs there. :-/


Good luck.

-- 
Pekka Paalanen
http://www.iki.fi/pq/

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Rafał Miłecki]

W dniu 18 czerwca 2011 12:39 użytkownik Pekka Paalanen <pq@iki.fi> napisał:
> On Sat, 18 Jun 2011 00:31:32 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> I use attached patch to fake result of read[bwl] performed by
>> closed source driver (ndiswrapper+bcmwl and wl).
>>
>> 1) It works great on my Sony VAIO with Intel(R) Core(TM)2 Duo CPU
>> P8400 2) It locks up Macbook Pro 8,1 with some 8-cores Intel
>>
>> Do you have any idea why it causes the lockup? Function causing
>> problem is "set_ins_reg_val". I've created it as copy of
>> get_ins_reg_val, it just sets values in struct pt_regs, instead of
>> reading them).
>
> Sorry, I have no insight to that... does unmodified mmiotrace
> work properly? Are you tracing the exact same kernel binary blob
> on both machines? Maybe it's using some rare instruction
> mmiotrace does not decode properly? Maybe with a rep prefix?
> Do those CPUs have any differences in their registers or
> struct pt_regs?
>
> I'm not even sure how "legal" it is to poke pt_regs there. :-/

Not modified MMIO tracing works great on this machine, I've grabbed
dumps 10-20 times without a lock up or anything.

I'm using different drivers on both machines, because Macbook Pro 8,1
has unique BCM4331 card that I can not buy and that is not available
with PCI(e) slot. Is uses some vendor specific, PCIe compatible slot.
Simple commenting out "set_ins_reg_val" work fine on this Macbook, PHY
reads are tracked correctly.

As for differences in struct pt_regs... yeah, I think that happens.
I'm using x86 kernel, while on Macbook we use x86_64 as it's required
to use 64bit driver in ndiswrapper.

I can try to find out, which register we try to overwrite on Macbook.

-- 
Rafał

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Rafał Miłecki]

W dniu 18 czerwca 2011 12:57 użytkownik Rafał Miłecki
<zajec5@gmail.com> napisał:
> W dniu 18 czerwca 2011 12:39 użytkownik Pekka Paalanen <pq@iki.fi> napisał:
>> On Sat, 18 Jun 2011 00:31:32 +0200
>> Rafał Miłecki <zajec5@gmail.com> wrote:
>>
>>> I use attached patch to fake result of read[bwl] performed by
>>> closed source driver (ndiswrapper+bcmwl and wl).
>>>
>>> 1) It works great on my Sony VAIO with Intel(R) Core(TM)2 Duo CPU
>>> P8400 2) It locks up Macbook Pro 8,1 with some 8-cores Intel
>>>
>>> Do you have any idea why it causes the lockup? Function causing
>>> problem is "set_ins_reg_val". I've created it as copy of
>>> get_ins_reg_val, it just sets values in struct pt_regs, instead of
>>> reading them).
>>
>> Sorry, I have no insight to that... does unmodified mmiotrace
>> work properly? Are you tracing the exact same kernel binary blob
>> on both machines? Maybe it's using some rare instruction
>> mmiotrace does not decode properly? Maybe with a rep prefix?
>> Do those CPUs have any differences in their registers or
>> struct pt_regs?
>>
>> I'm not even sure how "legal" it is to poke pt_regs there. :-/
>
> Not modified MMIO tracing works great on this machine, I've grabbed
> dumps 10-20 times without a lock up or anything.
>
> I'm using different drivers on both machines, because Macbook Pro 8,1
> has unique BCM4331 card that I can not buy and that is not available
> with PCI(e) slot. Is uses some vendor specific, PCIe compatible slot.
> Simple commenting out "set_ins_reg_val" work fine on this Macbook, PHY
> reads are tracked correctly.
>
> As for differences in struct pt_regs... yeah, I think that happens.
> I'm using x86 kernel, while on Macbook we use x86_64 as it's required
> to use 64bit driver in ndiswrapper.
>
> I can try to find out, which register we try to overwrite on Macbook.

This is what does happen on my machine (working):
[  122.550991] mmiotrace: ZAJEC: read PHY 0x20
[  122.550994] mmiotrace: ZAJEC: overwriting 0x20 with 0xFFFF
[  122.550997] [ZAJEC] setting AX with 0xFFFF
(...)
[  122.551071] mmiotrace: ZAJEC: read PHY 0x22
[  122.551074] mmiotrace: ZAJEC: overwriting 0x22 with 0xFFFF
[  122.551077] [ZAJEC] setting AX with 0xFFFF
(...)
[  122.551198] mmiotrace: ZAJEC: read PHY 0x27
[  122.551201] mmiotrace: ZAJEC: overwriting 0x27 with 0xFFFF
[  122.551204] [ZAJEC] setting AX with 0xFFFF


This is what does happen on Macbook:
[  166.886438] mmiotrace: ZAJEC: read PHY 0x810
[  166.886649] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
[  166.886860] [ZAJEC] setting AX with 0xFFFF
LOCK UP


So on both machines we modify AX register in the same place. My
function set_ins_reg_val is a copy of get_ins_reg_val which works
fine... So no idea what may we be doing wrong on this Macbook
x86_64...


P.S.
You can see I've changed 0x20, 0x22, 0x27 to 0x810 on Macbook. That's
because first 3 registers are not used by BCM4331. I've been testing
them with BCM43225.

-- 
Rafał

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Pekka Paalanen]

On Sat, 18 Jun 2011 13:26:14 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> W dniu 18 czerwca 2011 12:57 użytkownik Rafał Miłecki
> <zajec5@gmail.com> napisał:
> > Not modified MMIO tracing works great on this machine, I've
> > grabbed dumps 10-20 times without a lock up or anything.
> >
> > I'm using different drivers on both machines, because Macbook
> > Pro 8,1 has unique BCM4331 card that I can not buy and that is
> > not available with PCI(e) slot. Is uses some vendor specific,
> > PCIe compatible slot. Simple commenting out "set_ins_reg_val"
> > work fine on this Macbook, PHY reads are tracked correctly.
> >
> > As for differences in struct pt_regs... yeah, I think that
> > happens. I'm using x86 kernel, while on Macbook we use x86_64
> > as it's required to use 64bit driver in ndiswrapper.
> >
> > I can try to find out, which register we try to overwrite on
> > Macbook.
> 
> This is what does happen on my machine (working):
> [  122.550991] mmiotrace: ZAJEC: read PHY 0x20
> [  122.550994] mmiotrace: ZAJEC: overwriting 0x20 with 0xFFFF
> [  122.550997] [ZAJEC] setting AX with 0xFFFF
> (...)
> [  122.551071] mmiotrace: ZAJEC: read PHY 0x22
> [  122.551074] mmiotrace: ZAJEC: overwriting 0x22 with 0xFFFF
> [  122.551077] [ZAJEC] setting AX with 0xFFFF
> (...)
> [  122.551198] mmiotrace: ZAJEC: read PHY 0x27
> [  122.551201] mmiotrace: ZAJEC: overwriting 0x27 with 0xFFFF
> [  122.551204] [ZAJEC] setting AX with 0xFFFF
> 
> 
> This is what does happen on Macbook:
> [  166.886438] mmiotrace: ZAJEC: read PHY 0x810
> [  166.886649] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
> [  166.886860] [ZAJEC] setting AX with 0xFFFF
> LOCK UP
> 
> 
> So on both machines we modify AX register in the same place. My
> function set_ins_reg_val is a copy of get_ins_reg_val which works
> fine... So no idea what may we be doing wrong on this Macbook
> x86_64...

Ok, so it is a 32 vs. 64 bit arch difference, or difference in
driver binary. AX on 64-bit is actually RAX... well, depending
on data width.

I actually missed you patch attachment before, sorry.

I have minor notes, but I cannot see them being a reason for a
lockup:

- instead of set_reg_w32(), you should be able to simply
*get_reg_w32() = (unsigned long)value; or equivalent since
it returns a pointer.

- you are not checking the data access width, but you assume
it is 32 bits. Maybe you should verify that? get_reg_w8() is
very different. I think you should reproduce the switch on
get_ins_reg_width() statement from get_ins_reg_val() in
your set_ins_reg_val(), and use unsigned long instead of u32
to account for 64-bitness.

Yes, get_reg_w32() is a little badly named.

Maybe the driver is doing a 16-bit wide access, and happens to
store something else in the other 16/48 bits of RAX?

I assume the lockup is silent, since you have not shown
anything. Have you tried a serial console, if you have one?


HTH.

-- 
Pekka Paalanen
http://www.iki.fi/pq/

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Rafał Miłecki]

W dniu 18 czerwca 2011 14:03 użytkownik Pekka Paalanen <pq@iki.fi> napisał:
> On Sat, 18 Jun 2011 13:26:14 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> W dniu 18 czerwca 2011 12:57 użytkownik Rafał Miłecki
>> <zajec5@gmail.com> napisał:
>> > Not modified MMIO tracing works great on this machine, I've
>> > grabbed dumps 10-20 times without a lock up or anything.
>> >
>> > I'm using different drivers on both machines, because Macbook
>> > Pro 8,1 has unique BCM4331 card that I can not buy and that is
>> > not available with PCI(e) slot. Is uses some vendor specific,
>> > PCIe compatible slot. Simple commenting out "set_ins_reg_val"
>> > work fine on this Macbook, PHY reads are tracked correctly.
>> >
>> > As for differences in struct pt_regs... yeah, I think that
>> > happens. I'm using x86 kernel, while on Macbook we use x86_64
>> > as it's required to use 64bit driver in ndiswrapper.
>> >
>> > I can try to find out, which register we try to overwrite on
>> > Macbook.
>>
>> This is what does happen on my machine (working):
>> [  122.550991] mmiotrace: ZAJEC: read PHY 0x20
>> [  122.550994] mmiotrace: ZAJEC: overwriting 0x20 with 0xFFFF
>> [  122.550997] [ZAJEC] setting AX with 0xFFFF
>> (...)
>> [  122.551071] mmiotrace: ZAJEC: read PHY 0x22
>> [  122.551074] mmiotrace: ZAJEC: overwriting 0x22 with 0xFFFF
>> [  122.551077] [ZAJEC] setting AX with 0xFFFF
>> (...)
>> [  122.551198] mmiotrace: ZAJEC: read PHY 0x27
>> [  122.551201] mmiotrace: ZAJEC: overwriting 0x27 with 0xFFFF
>> [  122.551204] [ZAJEC] setting AX with 0xFFFF
>>
>>
>> This is what does happen on Macbook:
>> [  166.886438] mmiotrace: ZAJEC: read PHY 0x810
>> [  166.886649] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
>> [  166.886860] [ZAJEC] setting AX with 0xFFFF
>> LOCK UP
>>
>>
>> So on both machines we modify AX register in the same place. My
>> function set_ins_reg_val is a copy of get_ins_reg_val which works
>> fine... So no idea what may we be doing wrong on this Macbook
>> x86_64...
>
> Ok, so it is a 32 vs. 64 bit arch difference, or difference in
> driver binary. AX on 64-bit is actually RAX... well, depending
> on data width.
>
> I actually missed you patch attachment before, sorry.
>
> I have minor notes, but I cannot see them being a reason for a
> lockup:

Thanks for answer!


> - instead of set_reg_w32(), you should be able to simply
> *get_reg_w32() = (unsigned long)value; or equivalent since
> it returns a pointer.

Agree.


> - you are not checking the data access width, but you assume
> it is 32 bits. Maybe you should verify that? get_reg_w8() is
> very different. I think you should reproduce the switch on
> get_ins_reg_width() statement from get_ins_reg_val() in
> your set_ins_reg_val(), and use unsigned long instead of u32
> to account for 64-bitness.
>
> Yes, get_reg_w32() is a little badly named.
>
> Maybe the driver is doing a 16-bit wide access, and happens to
> store something else in the other 16/48 bits of RAX?

Nice comment, thanks! I've decided to print info about planned
overwrite, instead of really doing it. Plus few more debugging
messages suggested by David:
[  293.682242] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
[  293.687929] mmiotrace: [ZAJEC] ins at ffffc90010503b60: 0f b7 81 fe 03 00
[  293.693651] mmiotrace: [ZAJEC] p = (unsigned char *)ins_addr;
         p == 0xf
[  293.699379] mmiotrace: [ZAJEC] p += skip_prefix(p, &prf);
         p == 0xf
[  293.705116] mmiotrace: [ZAJEC] p += get_opcode(p, &opcode);
         p == 0x81
[  293.710815] mmiotrace: [ZAJEC] opcode is 0xb70f
[  293.716411] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
[  293.722062] mmiotrace: [ZAJEC] after checing opcode we decided to use reg 0x0
[  293.727698] mmiotrace: [ZAJEC] width is 4
[  293.733219] [ZAJEC] (not) setting AX with 0xFFFF

So the read's width is 4, that's 32bit. If this is 64bit arch we could
be overwritting another 32bits in AX register, right? Can this be our
issue?


> I assume the lockup is silent, since you have not shown
> anything. Have you tried a serial console, if you have one?

I think David has some USB debugging (what ever it means...). Using
console mode just printed commands done right before overwritting
register.

-- 
Rafał

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Rafał Miłecki]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

W dniu 18 czerwca 2011 14:03 użytkownik Pekka Paalanen <pq@iki.fi> napisał:
> Maybe the driver is doing a 16-bit wide access, and happens to
> store something else in the other 16/48 bits of RAX?

OK, attached is updated version of my patch. I think we can get some
clue from dmesgs with this patch applied.


My system (working):
[   74.472502] mmiotrace: ZAJEC: overwriting 0x27 with 0xFFFF
[   74.472511] mmiotrace: [ZAJEC] opcode is 0x8B
[   74.472515] mmiotrace: [ZAJEC] prf info: shorted:1; enlarged:0, rexr:0, rex:0
[   74.472517] mmiotrace: [ZAJEC] register is 0x0
[   74.472520] mmiotrace: [ZAJEC] overwritting 2-byte value 0x0514 with 0xFFFF
[   74.472523] mmiotrace: [ZAJEC] overwritting resulted in 0xFFFF

[   74.487081] mmiotrace: ZAJEC: overwriting 0x20 with 0xFFFF
[   74.487086] mmiotrace: [ZAJEC] opcode is 0x8B
[   74.487089] mmiotrace: [ZAJEC] prf info: shorted:1; enlarged:0, rexr:0, rex:0
[   74.487092] mmiotrace: [ZAJEC] register is 0x0
[   74.487095] mmiotrace: [ZAJEC] overwritting 2-byte value 0x427E with 0xFFFF
[   74.487097] mmiotrace: [ZAJEC] overwritting resulted in 0xFFFF


MacBook (with real overwritting commenet out!):
[  228.248715] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
[  228.254227] mmiotrace: [ZAJEC] opcode is 0xB70F
[  228.259784] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
[  228.265399] mmiotrace: [ZAJEC] register is 0x0
[  228.270955] mmiotrace: [ZAJEC] overwritting 4-byte value 0x00000000
with 0xFFFF
[  228.276597] mmiotrace: [ZAJEC] overwritting resulted in 0x00000000

[  228.284284] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
[  228.289818] mmiotrace: [ZAJEC] opcode is 0xB70F
[  228.295250] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
[  228.300838] mmiotrace: [ZAJEC] register is 0x0
[  228.306339] mmiotrace: [ZAJEC] overwritting 4-byte value 0x00000000
with 0xFFFF
[  228.311905] mmiotrace: [ZAJEC] overwritting resulted in 0x00000000


It's 2-byte vs. 4-byte. I suspect this can be the source of our
problem. Writing u16 0xFFFF value as u32 write.

-- 
Rafał

[-- Attachment #2: mmio.debugging.patch --]
[-- Type: application/octet-stream, Size: 4443 bytes --]

diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c
index 3adff7d..d5ac7da 100644
--- a/arch/x86/mm/mmio-mod.c
+++ b/arch/x86/mm/mmio-mod.c
@@ -206,6 +206,12 @@ static void pre(struct kmmio_probe *p, struct pt_regs *regs,
 	put_cpu_var(pf_reason);
 }
 
+#define MMIO_BASE			0xfaafc000
+#define B43_MMIO_PHY_CONTROL		0x3FC
+#define B43_MMIO_PHY_DATA		0x3FE
+
+static u16 broadcom_phy_addr;
+
 static void post(struct kmmio_probe *p, unsigned long condition,
 							struct pt_regs *regs)
 {
@@ -219,6 +225,19 @@ static void post(struct kmmio_probe *p, unsigned long condition,
 		BUG();
 	}
 
+	if (my_reason->type == REG_READ && my_trace->phys == MMIO_BASE + B43_MMIO_PHY_DATA) {
+		//pr_info("ZAJEC: read PHY 0x%X\n", broadcom_phy_addr);
+		switch (broadcom_phy_addr){
+		case 0x20:
+		case 0x22:
+		case 0x27:
+		case 0x810:
+			pr_info("ZAJEC: overwriting 0x%X with 0xFFFF\n", broadcom_phy_addr);
+			set_ins_reg_val(my_reason->ip, regs, 0xFFFF);
+			break;
+		}
+	}
+
 	switch (my_reason->type) {
 	case REG_READ:
 		my_trace->value = get_ins_reg_val(my_reason->ip, regs);
@@ -227,6 +246,11 @@ static void post(struct kmmio_probe *p, unsigned long condition,
 		break;
 	}
 
+	if (my_reason->type == REG_WRITE && my_trace->phys == MMIO_BASE + B43_MMIO_PHY_CONTROL) {
+		broadcom_phy_addr = my_trace->value;
+		//pr_info("ZAJEC: setting PHY addr to 0x%X\n", broadcom_phy_addr);
+	}
+
 	mmio_trace_rw(my_trace);
 	put_cpu_var(cpu_trace);
 	put_cpu_var(pf_reason);
diff --git a/arch/x86/mm/pf_in.c b/arch/x86/mm/pf_in.c
index 9f0614d..8519f69 100644
--- a/arch/x86/mm/pf_in.c
+++ b/arch/x86/mm/pf_in.c
@@ -461,6 +461,75 @@ err:
 	return 0;
 }
 
+void set_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs, u32 val)
+{
+	unsigned int opcode;
+	int reg;
+	unsigned char *p;
+	struct prefix_bits prf;
+	int i;
+
+	p = (unsigned char *)ins_addr;
+	p += skip_prefix(p, &prf);
+	p += get_opcode(p, &opcode);
+	for (i = 0; i < ARRAY_SIZE(reg_rop); i++)
+		if (reg_rop[i] == opcode)
+			goto do_work;
+
+	for (i = 0; i < ARRAY_SIZE(reg_wop); i++)
+		if (reg_wop[i] == opcode)
+			goto do_work;
+
+	printk(KERN_ERR "mmiotrace: Not a register instruction, opcode "
+							"0x%02x\n", opcode);
+	return;
+
+do_work:
+	printk(KERN_INFO "mmiotrace: [ZAJEC] opcode is 0x%X\n", opcode);
+	printk(KERN_INFO "mmiotrace: [ZAJEC] prf info: shorted:%d; enlarged:%d, rexr:%d, rex:%d\n", prf.shorted, prf.enlarged, prf.rexr, prf.rex);
+	/* for STOS, source register is fixed */
+	if (opcode == 0xAA || opcode == 0xAB) {
+		reg = arg_AX;
+	} else {
+		unsigned char mod_rm = *p;
+		reg = ((mod_rm >> 3) & 0x7) | (prf.rexr << 3);
+	}
+	printk(KERN_INFO "mmiotrace: [ZAJEC] register is 0x%X\n", reg);
+	switch (get_ins_reg_width(ins_addr)) {
+	case 1:
+		printk(KERN_ERR "mmiotrace: [ZAJEC] unsupported width 1\n");
+		break;
+	case 2:
+		{
+		unsigned short *zptr = (unsigned short *)get_reg_w32(reg, regs);
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting 2-byte value 0x%04X with 0x%X\n", *(zptr), val);
+		*(zptr) = val;
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting resulted in 0x%04X\n", *(zptr));
+		}
+		break;
+	case 4:
+		{
+		unsigned int *zptr = (unsigned int *)get_reg_w32(reg, regs);
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting 4-byte value 0x%08X with 0x%X\n", *(zptr), val);
+		*(zptr) = val;
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting resulted in 0x%08X\n", *(zptr));
+		}
+		break;
+#ifdef __amd64__
+	case 8:
+		{
+		unsigned long *zptr = (unsigned long *)get_reg_w32(reg, regs);
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting 8-byte value 0x%016X with 0x%X\n", *(zptr), val);
+		*(zptr) = val;
+		printk(KERN_INFO "mmiotrace: [ZAJEC] overwritting resulted in 0x%016lX\n", *(zptr));
+		}
+		break;
+#endif
+	default:
+		printk(KERN_ERR "mmiotrace: [ZAJEC] Error width# %d\n", reg);
+	}
+}
+
 unsigned long get_ins_imm_val(unsigned long ins_addr)
 {
 	unsigned int opcode;
diff --git a/arch/x86/mm/pf_in.h b/arch/x86/mm/pf_in.h
index e05341a..90b43ff 100644
--- a/arch/x86/mm/pf_in.h
+++ b/arch/x86/mm/pf_in.h
@@ -34,6 +34,7 @@ enum reason_type {
 enum reason_type get_ins_type(unsigned long ins_addr);
 unsigned int get_ins_mem_width(unsigned long ins_addr);
 unsigned long get_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs);
+void set_ins_reg_val(unsigned long ins_addr, struct pt_regs *regs, u32 val);
 unsigned long get_ins_imm_val(unsigned long ins_addr);
 
 #endif /* __PF_H_ */

Re: Lock up when faking MMIO read[bwl] on some machines [WAS: Faking MMIO ops? Fooling a driver]

[Rafał Miłecki]

W dniu 18 czerwca 2011 16:43 użytkownik Rafał Miłecki
<zajec5@gmail.com> napisał:
> W dniu 18 czerwca 2011 14:03 użytkownik Pekka Paalanen <pq@iki.fi> napisał:
>> Maybe the driver is doing a 16-bit wide access, and happens to
>> store something else in the other 16/48 bits of RAX?
>
> OK, attached is updated version of my patch. I think we can get some
> clue from dmesgs with this patch applied.
>
>
> My system (working):
> [   74.472502] mmiotrace: ZAJEC: overwriting 0x27 with 0xFFFF
> [   74.472511] mmiotrace: [ZAJEC] opcode is 0x8B
> [   74.472515] mmiotrace: [ZAJEC] prf info: shorted:1; enlarged:0, rexr:0, rex:0
> [   74.472517] mmiotrace: [ZAJEC] register is 0x0
> [   74.472520] mmiotrace: [ZAJEC] overwritting 2-byte value 0x0514 with 0xFFFF
> [   74.472523] mmiotrace: [ZAJEC] overwritting resulted in 0xFFFF
>
> [   74.487081] mmiotrace: ZAJEC: overwriting 0x20 with 0xFFFF
> [   74.487086] mmiotrace: [ZAJEC] opcode is 0x8B
> [   74.487089] mmiotrace: [ZAJEC] prf info: shorted:1; enlarged:0, rexr:0, rex:0
> [   74.487092] mmiotrace: [ZAJEC] register is 0x0
> [   74.487095] mmiotrace: [ZAJEC] overwritting 2-byte value 0x427E with 0xFFFF
> [   74.487097] mmiotrace: [ZAJEC] overwritting resulted in 0xFFFF
>
>
> MacBook (with real overwritting commenet out!):
> [  228.248715] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
> [  228.254227] mmiotrace: [ZAJEC] opcode is 0xB70F
> [  228.259784] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
> [  228.265399] mmiotrace: [ZAJEC] register is 0x0
> [  228.270955] mmiotrace: [ZAJEC] overwritting 4-byte value 0x00000000
> with 0xFFFF
> [  228.276597] mmiotrace: [ZAJEC] overwritting resulted in 0x00000000
>
> [  228.284284] mmiotrace: ZAJEC: overwriting 0x810 with 0xFFFF
> [  228.289818] mmiotrace: [ZAJEC] opcode is 0xB70F
> [  228.295250] mmiotrace: [ZAJEC] prf info: shorted:0; enlarged:0, rexr:0, rex:0
> [  228.300838] mmiotrace: [ZAJEC] register is 0x0
> [  228.306339] mmiotrace: [ZAJEC] overwritting 4-byte value 0x00000000
> with 0xFFFF
> [  228.311905] mmiotrace: [ZAJEC] overwritting resulted in 0x00000000
>
>
> It's 2-byte vs. 4-byte. I suspect this can be the source of our
> problem. Writing u16 0xFFFF value as u32 write.

Ohh, that was so stupid...

Writing 0xFFFF to PHY register 0x810 causes lockup on BCM4331! That's
all! My code is working fine, just some bits in 0xFFFF value are
really not friendly for the hardware.

-- 
Rafał