Re: [Xen-devel] [PATCH 1/1] amd/iommu: Fix infinite loop when handling IO_PAGE_FAULT event

[Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>]

On 12/29/2013 11:33 PM, Andrew Cooper wrote:
> On 29/12/2013 09:35, suravee.suthikulpanit@xxxxxxx wrote:
> > From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> >
> > Certain AMD systems could have upto 0x1000 ivrs_bdf_entries.
> > However, the loop variable (bdf) is declared as u16 which causes
> > inifinite loop when parsing IOMMU event log with IO_PAGE_FAULT event.
> > This patch changes the variable to u32 instead.
> Do you perhaps mean that there could be 0x10000 ivrs_bdf_entries?
> Otherwise I cant see how an infinite loop is possible.
Ah Yes, This is actually 0x10000. Sorry for the typo.

> On the other hand, assuming that the infinite loop is possible, it is
> also vulnerable in register_exclusion_range_for_{all,iommu}_devices(),
> which also have similar for loops with a u16 bdf.
Thanks for catching the rest here.  I'll clean them up also and send out V2.

> Even if you do promote to a u32, the get_dma_requestor_id() call now
> truncates a u32 to a u16, so can now return the wrong device.
Actually, bdf should only be 16 bits. However, I think we just need to 
resolve the looping logic.  The truncation should not cause issue here.
> Beyond that, there is already quite a mix of u32, int and u16's for
> various bdf values across the this area of the code, with plenty of
> truncation issues at a glance.
>
> ~Andrew
I'll try to go through them and clean up in V2.

Suravee


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel