drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported()
Subsystems (3)
PATCH 1/1 drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported()
The "block" variable can be set by the user through debugfs, so it can be quite large which leads to shift wrapping here. This means we report a "block" as supported when it's not, and that leads to array overflows later on.
This bug is not really a security issue in real life, because debugfs is generally root only.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h +2 -0
@@ -173,6 +173,8 @@ static inline int amdgpu_ras_is_supported(struct amdgpu_device *adev, {struct amdgpu_ras *ras =amdgpu_ras_get_context (adev); +if (block >=AMDGPU_RAS_BLOCK_COUNT ) +return 0 ;return ras && (ras->supported & (1 << block)); }
Signature · 6 lines
2.20.1 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
do you mean that something like 1<<65 might be a none zero value?
________________________________
From: Dan Carpenter <[email protected]>
Sent: Saturday, June 8, 2019 5:23:57 PM
To: Deucher, Alexander; Pan, Xinhui
Cc: Koenig, Christian; Zhou, David(ChunMing); David Airlie; Daniel Vetter; Quan, Evan; Zhu, James; [email protected]; [email protected]; [email protected]
Subject: [PATCH] drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported()
The "block" variable can be set by the user through debugfs, so it can be quite large which leads to shift wrapping here. This means we report a "block" as supported when it's not, and that leads to array overflows later on.
This bug is not really a security issue in real life, because debugfs is generally root only.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h +2 -0
@@ -173,6 +173,8 @@ static inline int amdgpu_ras_is_supported(struct amdgpu_device *adev, {struct amdgpu_ras *ras =amdgpu_ras_get_context (adev); +if (block >=AMDGPU_RAS_BLOCK_COUNT ) +return 0 ;return ras && (ras->supported & (1 << block)); }
--
2.20.1
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Koenig, Christian
maintainerYes, that is undefined behavior what you do here.
See here as well https://stackoverflow.com/questions/11270492/what-does-the-c-standard-say-about-bitshifting-more-bits-than-the-width-of-type.
Christian.
Am 08.06.19 um 14:27 schrieb Pan, Xinhui:
do you mean that something like 1<<65 might be a none zero value?
________________________________
From: Dan Carpenter <[email protected]><mailto:[email protected]>
Sent: Saturday, June 8, 2019 5:23:57 PM
To: Deucher, Alexander; Pan, Xinhui
Cc: Koenig, Christian; Zhou, David(ChunMing); David Airlie; Daniel Vetter; Quan, Evan; Zhu, James; [email protected]<mailto:[email protected]>; [email protected]<mailto:[email protected]>; [email protected]<mailto:[email protected]>
Subject: [PATCH] drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported()
The "block" variable can be set by the user through debugfs, so it can be quite large which leads to shift wrapping here. This means we report a "block" as supported when it's not, and that leads to array overflows later on.
This bug is not really a security issue in real life, because debugfs is generally root only.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h +2 -0
@@ -173,6 +173,8 @@ static inline int amdgpu_ras_is_supported(struct amdgpu_device *adev, {struct amdgpu_ras *ras =amdgpu_ras_get_context (adev); +if (block >=AMDGPU_RAS_BLOCK_COUNT ) +return 0 ;return ras && (ras->supported & (1 << block)); }
--
2.20.1
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel