On 25.08.2010, at 10:48, Heiko Carstens wrote:
> 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.. :)
Thinking about this a bit more ... it's no problem to access beyond the device page. After the device page follow the rings and we always have rings because we always have a virtio console :).
Alex
--
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
