[01/10] drm/amdgpu: set ip specific ras interface pointer to NULL after free it

Submitted by Hawking Zhang on Sept. 3, 2019, 12:01 a.m.

Details

Message ID 1567468894-11852-1-git-send-email-Hawking.Zhang@amd.com
State Accepted
Commit d094aea312580f12232b546523dae20f54445469
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Hawking Zhang Sept. 3, 2019, 12:01 a.m.
to prevent access to dangling pointers

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 7 +++++--
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 8 ++++++--
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 4 +++-
 drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c  | 4 ++++
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c  | 8 ++++++--
 5 files changed, 24 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index cb8d8b6..c902885 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -4431,14 +4431,17 @@  static int gfx_v9_0_ecc_late_init(void *handle)
 		r = amdgpu_irq_get(adev, &adev->gfx.cp_ecc_error_irq, 0);
 		if (r)
 			goto late_fini;
-	} else
-		kfree(adev->gfx.ras_if);
+	} else {
+		r = 0;
+		goto free;
+	}
 
 	return 0;
 late_fini:
 	amdgpu_ras_late_fini(adev, adev->gfx.ras_if, &ih_info);
 free:
 	kfree(adev->gfx.ras_if);
+	adev->gfx.ras_if = NULL;
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index af069a4..74483a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -793,8 +793,11 @@  static int gmc_v9_0_ecc_late_init(void *handle)
 		r = amdgpu_irq_get(adev, &adev->gmc.ecc_irq, 0);
 		if (r)
 			goto umc_late_fini;
-	} else
-		kfree(adev->gmc.umc_ras_if);
+	} else {
+		/* free umc ras_if if umc ras is not supported */
+		r = 0;
+		goto free;
+	}
 
 	if (adev->mmhub_funcs && adev->mmhub_funcs->ras_late_init) {
 		r = adev->mmhub_funcs->ras_late_init(adev);
@@ -806,6 +809,7 @@  static int gmc_v9_0_ecc_late_init(void *handle)
 	amdgpu_ras_late_fini(adev, adev->gmc.umc_ras_if, &umc_ih_info);
 free:
 	kfree(adev->gmc.umc_ras_if);
+	adev->gmc.umc_ras_if = NULL;
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index ab6559a..9916a33 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -639,8 +639,10 @@  static int mmhub_v1_0_ras_late_init(struct amdgpu_device *adev)
 	mmhub_ih_info.head = mmhub_fs_info.head = *adev->gmc.mmhub_ras_if;
 	r = amdgpu_ras_late_init(adev, adev->gmc.mmhub_ras_if,
 				 &mmhub_fs_info, &mmhub_ih_info);
-	if (r || !amdgpu_ras_is_supported(adev, adev->gmc.mmhub_ras_if->block))
+	if (r || !amdgpu_ras_is_supported(adev, adev->gmc.mmhub_ras_if->block)) {
 		kfree(adev->gmc.mmhub_ras_if);
+		adev->gmc.mmhub_ras_if = NULL;
+	}
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
index 5e784bb..76c0029 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
@@ -502,6 +502,9 @@  static int nbio_v7_4_ras_late_init(struct amdgpu_device *adev)
 		r = amdgpu_irq_get(adev, &adev->nbio.ras_err_event_athub_irq, 0);
 		if (r)
 			goto late_fini;
+	} else {
+		r = 0;
+		goto free;
 	}
 
 	return 0;
@@ -509,6 +512,7 @@  static int nbio_v7_4_ras_late_init(struct amdgpu_device *adev)
 	amdgpu_ras_late_fini(adev, adev->nbio.ras_if, &ih_info);
 free:
 	kfree(adev->nbio.ras_if);
+	adev->nbio.ras_if = NULL;
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index df2fb1f..e971e86 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1725,14 +1725,18 @@  static int sdma_v4_0_late_init(void *handle)
 			if (r)
 				goto late_fini;
 		}
-	} else
-		kfree(adev->sdma.ras_if);
+	} else {
+		/* free sdma ras_if if sdma ras is not supported */
+		r = 0;
+		goto free;
+	}
 
         return 0;
 late_fini:
 	amdgpu_ras_late_fini(adev, adev->sdma.ras_if, &ih_info);
 free:
 	kfree(adev->sdma.ras_if);
+	adev->sdma.ras_if = NULL;
 	return r;
 }
 

Comments

I think ecc_irq and process_ras_data_cb functions could be also moved to generic module files, but we can do it in new patches.

Another issue is amdgpu init will fail if vbios enables ras but psp ras ta is not ready on Arcturus. If vbios and psp ta for ras will be released at the same time, the series is:

Reviewed-by: Tao Zhou <tao.zhou1@amd.com>


> -----Original Message-----

> From: Zhang, Hawking <Hawking.Zhang@amd.com>

> Sent: 2019年9月3日 8:02

> To: amd-gfx@lists.freedesktop.org; Zhou1, Tao <Tao.Zhou1@amd.com>;

> Chen, Guchun <Guchun.Chen@amd.com>; Deucher, Alexander

> <Alexander.Deucher@amd.com>

> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>

> Subject: [PATCH 01/10] drm/amdgpu: set ip specific ras interface pointer to

> NULL after free it

> 

> to prevent access to dangling pointers

> 

> Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>

> ---

>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 7 +++++--

>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 8 ++++++--

>  drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 4 +++-

> drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c  | 4 ++++

> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c  | 8 ++++++--

>  5 files changed, 24 insertions(+), 7 deletions(-)

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

> index cb8d8b6..c902885 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

> @@ -4431,14 +4431,17 @@ static int gfx_v9_0_ecc_late_init(void *handle)

>  		r = amdgpu_irq_get(adev, &adev->gfx.cp_ecc_error_irq, 0);

>  		if (r)

>  			goto late_fini;

> -	} else

> -		kfree(adev->gfx.ras_if);

> +	} else {

> +		r = 0;

> +		goto free;

> +	}

> 

>  	return 0;

>  late_fini:

>  	amdgpu_ras_late_fini(adev, adev->gfx.ras_if, &ih_info);

>  free:

>  	kfree(adev->gfx.ras_if);

> +	adev->gfx.ras_if = NULL;

>  	return r;

>  }

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> index af069a4..74483a7 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> @@ -793,8 +793,11 @@ static int gmc_v9_0_ecc_late_init(void *handle)

>  		r = amdgpu_irq_get(adev, &adev->gmc.ecc_irq, 0);

>  		if (r)

>  			goto umc_late_fini;

> -	} else

> -		kfree(adev->gmc.umc_ras_if);

> +	} else {

> +		/* free umc ras_if if umc ras is not supported */

> +		r = 0;

> +		goto free;

> +	}

> 

>  	if (adev->mmhub_funcs && adev->mmhub_funcs->ras_late_init) {

>  		r = adev->mmhub_funcs->ras_late_init(adev);

> @@ -806,6 +809,7 @@ static int gmc_v9_0_ecc_late_init(void *handle)

>  	amdgpu_ras_late_fini(adev, adev->gmc.umc_ras_if, &umc_ih_info);

>  free:

>  	kfree(adev->gmc.umc_ras_if);

> +	adev->gmc.umc_ras_if = NULL;

>  	return r;

>  }

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c

> b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c

> index ab6559a..9916a33 100644

> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c

> @@ -639,8 +639,10 @@ static int mmhub_v1_0_ras_late_init(struct

> amdgpu_device *adev)

>  	mmhub_ih_info.head = mmhub_fs_info.head = *adev-

> >gmc.mmhub_ras_if;

>  	r = amdgpu_ras_late_init(adev, adev->gmc.mmhub_ras_if,

>  				 &mmhub_fs_info, &mmhub_ih_info);

> -	if (r || !amdgpu_ras_is_supported(adev, adev->gmc.mmhub_ras_if-

> >block))

> +	if (r || !amdgpu_ras_is_supported(adev,

> +adev->gmc.mmhub_ras_if->block)) {

>  		kfree(adev->gmc.mmhub_ras_if);

> +		adev->gmc.mmhub_ras_if = NULL;

> +	}

>  	return r;

>  }

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c

> b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c

> index 5e784bb..76c0029 100644

> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c

> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c

> @@ -502,6 +502,9 @@ static int nbio_v7_4_ras_late_init(struct

> amdgpu_device *adev)

>  		r = amdgpu_irq_get(adev, &adev-

> >nbio.ras_err_event_athub_irq, 0);

>  		if (r)

>  			goto late_fini;

> +	} else {

> +		r = 0;

> +		goto free;

>  	}

> 

>  	return 0;

> @@ -509,6 +512,7 @@ static int nbio_v7_4_ras_late_init(struct

> amdgpu_device *adev)

>  	amdgpu_ras_late_fini(adev, adev->nbio.ras_if, &ih_info);

>  free:

>  	kfree(adev->nbio.ras_if);

> +	adev->nbio.ras_if = NULL;

>  	return r;

>  }

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

> index df2fb1f..e971e86 100644

> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

> @@ -1725,14 +1725,18 @@ static int sdma_v4_0_late_init(void *handle)

>  			if (r)

>  				goto late_fini;

>  		}

> -	} else

> -		kfree(adev->sdma.ras_if);

> +	} else {

> +		/* free sdma ras_if if sdma ras is not supported */

> +		r = 0;

> +		goto free;

> +	}

> 

>          return 0;

>  late_fini:

>  	amdgpu_ras_late_fini(adev, adev->sdma.ras_if, &ih_info);

>  free:

>  	kfree(adev->sdma.ras_if);

> +	adev->sdma.ras_if = NULL;

>  	return r;

>  }

> 

> --

> 2.7.4