[PATCH xf86-video-amdgpu] Fix crash in PCI probe path (v2)

Michel Dänzer michel at daenzer.net
Wed Oct 28 01:03:22 PDT 2015


On 27.10.2015 11:23, Jammy Zhou wrote:
> The crash is caused by the NULL value returned by AMDGPUPTR(pScrn),
> because the driverPrivate is not allocated yet in PciProbe phase,
> and it is usually done in the PreInit phase.
> 
> Use pAMDGPUEnt->fd instead of info->dri2.drm_fd to avoid AMDGPUInfoPtr
> related code in amdgpu_open_drm_master, so that the crash can be fixed.
> 
> v2: switch to pAMDGPUEnt->fd, and update the commit message
> 
> Change-Id: I817c5f2324780b1b3ab51a366192c11ef4fff990
> Signed-off-by: Jammy Zhou <Jammy.Zhou at amd.com>
> ---
>  src/amdgpu_probe.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
> index 481271b..ae78233 100644
> --- a/src/amdgpu_probe.c
> +++ b/src/amdgpu_probe.c
> @@ -147,10 +147,10 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn, struct pci_device *dev,
>  	return fd;
>  }
>  
> -static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn)
> +static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn,
> +				   AMDGPUEntPtr pAMDGPUEnt, int entity_num)
>  {
> -	AMDGPUInfoPtr  info   = AMDGPUPTR(pScrn);
> -	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
> +	struct pci_device *dev;

Please make amdgpu_open_drm_master take the struct pci_device pointer as
a parameter and pass pci_dev for that in amdgpu_get_scrninfo.


> @@ -158,13 +158,13 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn)
>  		xf86DrvMsg(pScrn->scrnIndex, X_INFO,
>  			   " reusing fd for second head\n");
>  
> -		info->drmmode.fd = info->dri2.drm_fd = pAMDGPUEnt->fd;
>  		pAMDGPUEnt->fd_ref++;
>  		return TRUE;
>  	}

This whole block is dead code (amdgpu_get_scrninfo allocates the memory
pointed to by pAMDGPUEnt just before calling amdgpu_open_drm_master, so
pAMDGPUEnt->fd is always 0), please remove it.


Other than that, the patch looks good. The pAMDGPUEnt->fd = 0 assignment
under the error_amdgpu label could be removed as well, but I'm fine with
leaving it.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer



More information about the xorg-driver-ati mailing list