From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulpanit Subject: Re: [PATCH 1/1] amd/iommu: Fix infinite loop when handling IO_PAGE_FAULT event Date: Sun, 29 Dec 2013 22:51:30 +0700 Message-ID: <52C04502.8000304@amd.com> References: <1388309750-4495-1-git-send-email-suravee.suthikulpanit@amd.com> <52C04EC0.4010406@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52C04EC0.4010406@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , JBeulich@suse.com Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 12/29/2013 11:33 PM, Andrew Cooper wrote: > On 29/12/2013 09:35, suravee.suthikulpanit@amd.com wrote: >> From: Suravee Suthikulpanit >> >> 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