| Message ID | 20241004235904.8605-1-pvmohammedanees2003@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amdgpu: prevent BO_HANDLES error from being overwritten | expand |
Hi, Le 05/10/2024 à 01:59, Mohammed Anees a écrit : > Before this patch, if multiple BO_HANDLES chunks were submitted, > the error -EINVAL would be correctly set but could be overwritten > by the return value from amdgpu_cs_p1_bo_handles(). This patch > ensures that once an error condition is detected, the function > returns immediately, avoiding the overwriting of the error code. > > Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 1e475eb01417..543db0df9a31 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -266,8 +266,9 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p, > /* Only a single BO list is allowed to simplify handling. */ > if (p->bo_list) > ret = -EINVAL; > + else > + ret = amdgpu_cs_p1_bo_handles(p, p->chunks[i].kdata); > > - ret = amdgpu_cs_p1_bo_handles(p, p->chunks[i].kdata); My bad, I've merged the wrong version of the patch. That being said, I think it'd be nicer to follow the same pattern as the other checks and do: if (p->bo_list) goto free_partial_kdata; Since "ret" is initialized to -EINVAL already. Also can you add a reference to the broken commit in the commit message? Something like: Fixes: fec5f8e8c6bcf83 ("drm/amdgpu: disallow multiple BO_HANDLES chunks in one submit") Thanks, Pierre-Eric > if (ret) > goto free_partial_kdata; > break;
Am 09.10.24 um 10:22 schrieb Pierre-Eric Pelloux-Prayer: > Hi, > > Le 05/10/2024 à 01:59, Mohammed Anees a écrit : >> Before this patch, if multiple BO_HANDLES chunks were submitted, >> the error -EINVAL would be correctly set but could be overwritten >> by the return value from amdgpu_cs_p1_bo_handles(). This patch >> ensures that once an error condition is detected, the function >> returns immediately, avoiding the overwriting of the error code. >> >> Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 1e475eb01417..543db0df9a31 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -266,8 +266,9 @@ static int amdgpu_cs_pass1(struct >> amdgpu_cs_parser *p, >> /* Only a single BO list is allowed to simplify >> handling. */ >> if (p->bo_list) >> ret = -EINVAL; >> + else >> + ret = amdgpu_cs_p1_bo_handles(p, p->chunks[i].kdata); >> - ret = amdgpu_cs_p1_bo_handles(p, p->chunks[i].kdata); > > My bad, I've merged the wrong version of the patch. > > That being said, I think it'd be nicer to follow the same pattern as > the other checks and do: > > if (p->bo_list) > goto free_partial_kdata; > > Since "ret" is initialized to -EINVAL already. > > Also can you add a reference to the broken commit in the commit message? > Something like: > > Fixes: fec5f8e8c6bcf83 ("drm/amdgpu: disallow multiple BO_HANDLES > chunks in one submit") Yeah agree with Pierre-Eric and also please add the same CC stable tag the original patch had so that it gets backported ASAP. Thanks, Christian. > > Thanks, > Pierre-Eric > >> if (ret) >> goto free_partial_kdata; >> break;
Hi, I have sent the v2 Patch reflecting these changes along with the Cc tag as suggested by Christian. Thanks
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 1e475eb01417..543db0df9a31 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -266,8 +266,9 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p, /* Only a single BO list is allowed to simplify handling. */ if (p->bo_list) ret = -EINVAL; + else + ret = amdgpu_cs_p1_bo_handles(p, p->chunks[i].kdata); - ret = amdgpu_cs_p1_bo_handles(p, p->chunks[i].kdata); if (ret) goto free_partial_kdata; break;
Before this patch, if multiple BO_HANDLES chunks were submitted, the error -EINVAL would be correctly set but could be overwritten by the return value from amdgpu_cs_p1_bo_handles(). This patch ensures that once an error condition is detected, the function returns immediately, avoiding the overwriting of the error code. Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)