| Message ID | 0d4b92ab-f7c2-4f18-f3c3-c0f82ba47fc8@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd: Adjustments for three function implementations | expand |
Am 2023-04-11 um 09:42 schrieb Markus Elfring: > Date: Tue, 11 Apr 2023 10:52:48 +0200 > > The address of a data structure member was determined before > a corresponding null pointer check in the implementation of > the function “amdgpu_ras_interrupt_dispatch”. > > Thus avoid the risk for undefined behaviour by moving the assignment > for the variable “data” behind the null pointer check. > > This issue was detected by using the Coccinelle software. > > Fixes: c030f2e4166c3f5597c7e7a70bcd9ab383695de4 ("drm/amdgpu: add amdgpu_ras.c to support ras (v2)") > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > index 4069bce9479f..a920c7888d07 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > @@ -1730,11 +1730,12 @@ int amdgpu_ras_interrupt_dispatch(struct amdgpu_device *adev, > struct ras_dispatch_if *info) > { > struct ras_manager *obj = amdgpu_ras_find_obj(adev, &info->head); > - struct ras_ih_data *data = &obj->ih_data; > + struct ras_ih_data *data; I'm curious, this only takes the address of obj->ih_data. It doesn't dereference the pointer until after the !obj check below. How is this undefined behaviour? Is this about the compiler being free to reorder stuff for optimization, unaware of the dependency? Is there a link to an explanation that could be added to the commit description? Thanks, Felix > > if (!obj) > return -EINVAL; > > + data = &obj->ih_data; > if (data->inuse == 0) > return 0; > > -- > 2.40.0 >
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >> @@ -1730,11 +1730,12 @@ int amdgpu_ras_interrupt_dispatch(struct amdgpu_device *adev, >> struct ras_dispatch_if *info) >> { >> struct ras_manager *obj = amdgpu_ras_find_obj(adev, &info->head); >> - struct ras_ih_data *data = &obj->ih_data; >> + struct ras_ih_data *data; > I'm curious, this only takes the address of obj->ih_data. Even if a null pointer would accidentally be returned by a call of the function “amdgpu_ras_find_obj”? https://elixir.bootlin.com/linux/v6.3-rc6/source/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c#L618 > It doesn't dereference the pointer until after the !obj check below. Does the used arrow operator indicate a pointer dereference? > How is this undefined behaviour? I guess that another information source can be helpful for such an issue. https://wiki.sei.cmu.edu/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers?focusedCommentId=405504153#comment-405504153 Regards, Markus
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 4069bce9479f..a920c7888d07 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1730,11 +1730,12 @@ int amdgpu_ras_interrupt_dispatch(struct amdgpu_device *adev, struct ras_dispatch_if *info) { struct ras_manager *obj = amdgpu_ras_find_obj(adev, &info->head); - struct ras_ih_data *data = &obj->ih_data; + struct ras_ih_data *data; if (!obj) return -EINVAL; + data = &obj->ih_data; if (data->inuse == 0) return 0;
Date: Tue, 11 Apr 2023 10:52:48 +0200 The address of a data structure member was determined before a corresponding null pointer check in the implementation of the function “amdgpu_ras_interrupt_dispatch”. Thus avoid the risk for undefined behaviour by moving the assignment for the variable “data” behind the null pointer check. This issue was detected by using the Coccinelle software. Fixes: c030f2e4166c3f5597c7e7a70bcd9ab383695de4 ("drm/amdgpu: add amdgpu_ras.c to support ras (v2)") Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.40.0