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.. :)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
