[PATCH xf86-video-amdgpu 1/2] Fix crash in PCI probe path (v3)

[Michel Dänzer]

On 28.10.2015 22:24, 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.
> 
> v3: some more cleanup
> v2: switch to pAMDGPUEnt->fd, and update the commit message
> 
> Signed-off-by: Jammy Zhou <Jammy.Zhou at amd.com>
> ---
>  src/amdgpu_probe.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
> index 481271b..f6e675c 100644
> --- a/src/amdgpu_probe.c
> +++ b/src/amdgpu_probe.c
> @@ -147,24 +147,14 @@ 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,
> +				   struct pci_device *pci_dev, int entity_num)

Sorry I wasn't explicit about this before, but please don't add an
unused entity_num parameter.


> -	if (pAMDGPUEnt->fd) {
> -		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;
> -	}

You could rebase on top of the attached patch, since this is a logically
separate change from the crash fix.


> -static Bool amdgpu_get_scrninfo(int entity_num, void *pci_dev)
> +static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
>  {
>  	ScrnInfoPtr pScrn = NULL;
>  	EntityInfoPtr pEnt;
[...]
> @@ -252,8 +244,6 @@ static Bool amdgpu_get_scrninfo(int entity_num, void *pci_dev)
>  				   "amdgpu_device_initialize failed\n");
>  			goto error_amdgpu;
>  		}
> -	} else {
> -		pAMDGPUEnt = pPriv->ptr;
>  	}
>  
>  	xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
> @@ -266,7 +256,6 @@ static Bool amdgpu_get_scrninfo(int entity_num, void *pci_dev)
>  
>  error_amdgpu:
>  	drmClose(pAMDGPUEnt->fd);
> -	pAMDGPUEnt->fd = 0;
>  error_fd:
>  	free(pPriv->ptr);
>  	return FALSE;
> @@ -276,7 +265,7 @@ static Bool
>  amdgpu_pci_probe(DriverPtr pDriver,
>  		 int entity_num, struct pci_device *device, intptr_t match_data)
>  {
> -	return amdgpu_get_scrninfo(entity_num, (void *)device);
> +	return amdgpu_get_scrninfo(entity_num, device);
>  }
>  
>  static Bool AMDGPUDriverFunc(ScrnInfoPtr scrn, xorgDriverFuncOp op, void *data)
> 

These changes make sense, but are again logically separate from the
crash fix.

If you want, I can send out a new series with the logical changes split
up into separate patches.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Remove-dead-code-from-amdgpu_open_drm_master.patch
Type: text/x-patch
Size: 1204 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-driver-ati/attachments/20151029/d01725a6/attachment.bin>