On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
> On 25.08.2010, at 10:35, Heiko Carstens wrote:
> > On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
> >> On 25.08.2010, at 10:16, Heiko Carstens wrote:
> >>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> >>>> +static void hotplug_devices(struct work_struct *dummy)
> >>>> +{
> >>>> +        unsigned int i;
> >>>> +        struct kvm_device_desc *d;
> >>>> +        struct device *dev;
> >>>> +
> >>>> +        for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> >>> 
> >>> This should be 
> >>> 
> >>>   for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> >>> 
> >>> otherwise you might have memory accesses beyond the device page...
> >> 
> >> Oh, this is a simple copy&paste from the original search method.
> >> Is d valid in the first part of the loop already?
> > 
> > No, you would need to initialize it with kvm_devices if you change
> > the loop.
> 
> But even then it would take the size of the current entry, not the next
> one we're actually looking for, no? Maybe it'd be easier to just convert
> this into a while loop and explicitly open code everything.

Yes indeed. It's going to be quite ugly if you want to make sure that at
no time you're going to access memory beyond the device page. Eeek.. :)
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Reply via email to