Michel Dänzer wrote:
> On Wed, 2009-08-05 at 11:01 +0200, Thomas Hellström wrote:
>   
>> Michel Dänzer wrote:
>>     
>>> On Wed, 2009-08-05 at 10:50 +0200, Thomas Hellström wrote:
>>>   
>>>       
>>>> Michel Dänzer wrote:
>>>>     
>>>>         
>>>>> From: Michel Dänzer <daen...@vmware.com>
>>>>>
>>>>> Make sure bo->vm_node is valid, to prevent crashes in the fault handler, 
>>>>> and
>>>>> adjust vma->vm_pgoff according to it, so userspace attempts to access 
>>>>> /dev/fb*
>>>>> mappings don't always result in SIGBUS.
>>>>>
>>>>> Signed-off-by: Michel Dänzer <daen...@vmware.com>
>>>>> ---
>>>>>  drivers/gpu/drm/ttm/ttm_bo_vm.c |    6 ++++++
>>>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c 
>>>>> b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> index 27b146c..57d0c94 100644
>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> @@ -282,7 +282,13 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, 
>>>>> struct ttm_buffer_object *bo)
>>>>>   if (vma->vm_pgoff != 0)
>>>>>           return -EACCES;
>>>>>  
>>>>> + if (!bo->vm_node) {
>>>>> +         printk(KERN_ERR TTM_PFX "bo->vm_node == NULL\n");
>>>>> +         return -EACCES;
>>>>> + }
>>>>>   
>>>>>       
>>>>>           
>>>> Michel,
>>>> Did you actually hit a bo with an invalid vm_node?
>>>>     
>>>>         
>>> See patch 3 - the radeon driver used to create the fbcon BO as
>>> kernel-only, so when I tried passing that to ttm_fbdev_mmap()
>>> all /dev/fb* userspace mappings mysteriously caught SIGBUS on access.
>>>       
>
> Actually, it was even worse - the TTM fault handler crashed because it
> unconditionally dereferences bo->vm_node.
>
>   
>>> It took me a while to track that down, this is basically a debugging aid
>>> for bugs like that.
>>>   
>>>       
>> Ah, OK. But since that is really a kernel bug, we should trap it with 
>> BUG_ON().
>>     
>
> Good point, will do.
>
>
>   
Aargh. Wait. I remember now.

The fbcon bo is exported through the fbdev address space at offset 0.
The vm_node is for the drm device address space only. So it is perfectly 
legal and actually correct for it not to have a vm_node, unless it's 
going to be accessible from the drm device. Does it need to for KMS?

I'm a bit unsure whether it's OK to export a bo through two different 
address spaces. In particular, unmap_mapping_range() on the drm device 
will not kill the fbdev user-space mappings.

 The bug is in the ttm fbdev fault handler which is the same as the drm 
device fault handler that uses the vm_node->offset when it should 
actually use 0. We should probably add an is_fbdev argument to the fault 
handler and wrap it for its two uses.

/Thomas




------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to