Messages in this thread | | | Date | Fri, 14 Mar 2008 10:18:09 +0000 | From | "Jan Beulich" <> | Subject | Re: [PATCH] avoid endless loops in lib/swiotlb.c |
| |
>>> FUJITA Tomonori <tomof@acm.org> 14.03.08 10:35 >>> >On Thu, 13 Mar 2008 09:13:30 +0000 >"Jan Beulich" <jbeulich@novell.com> wrote: > >> Commit 681cc5cd3efbeafca6386114070e0bfb5012e249 introduced two >> possibilities for entering an endless loop in lib/swiotlb.c: >> >> - if max_slots is zero (possible if mask is ~0UL) > >Yeah, max_slots can not be zero. There are no drivers that have such >mask. I'm not sure that we will have.
You mean no current, in-tree drivers do this, and you imply the code is only used on 64-bits. Since Xen uses this file even for 32-bits, I had run into the case, and there's nothing preventing an in-tree driver from setting the mask to ~0UL, which would then result in a perhaps difficult to debug hang. For that reason, it seems better to me to fix this latent bug right away.
>... > >> - if the number of slots requested fits into a swiotlb segment, but is >> too large for the part of a segment which remains after considering >> offset_slots > >Sorry, I'm not sure what you mean. Can you give me an actual example >numbers that leads to that?
For one part, it can happen if nslots > max_slots (which is a driver error [except for the case above where max_slots erroneously got set to zero], but shouldn't lead to a silent hang, especially as it didn't do so before).
For another part, requesting e.g. a transfer of 128k with a segment mask of 128k when the IOTLB isn't aligned to a 128k boundary would again lead to a silent hang, as would various cases where the request exceeds the segment size (and the segment mask is sufficiently small). Neither of these cases got stuck in the old code.
Beyond that, maybe I was too quick in concluding this could happen even in less unusual cases - I think I didn't pay close enough attention to the fact that offset_slots + index gets masked by max_slots - 1. But even then I think the code looks simpler/safer and is smaller with the adjusted logic.
Jan
| |