Am 31.07.20 um 08:53 schrieb Greg Kroah-Hartman:
On Thu, Jul 30, 2020 at 05:09:07PM -0400, Luben Tuikov wrote:
On 2020-07-29 9:49 a.m., Alex Deucher wrote:
On Wed, Jul 29, 2020 at 4:11 AM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
Am 28.07.20 um 21:29 schrieb Peilin Ye:
Compiler leaves a 4-byte hole near the end of `dev_info`, causing
amdgpu_info_ioctl() to copy uninitialized kernel stack memory to userspace
when `size` is greater than 356.
In 2015 we tried to fix this issue by doing `= {};` on `dev_info`, which
unfortunately does not initialize that 4-byte hole. Fix it by using
memset() instead.
Cc: stable@xxxxxxxxxxxxxxx
Fixes: c193fa91b918 ("drm/amdgpu: information leak in amdgpu_info_ioctl()")
Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Suggested-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Peilin Ye <yepeilin.cs@xxxxxxxxx>
Reviewed-by: Christian König <christian.koenig@xxxxxxx>
I can't count how many of those we have fixed over the years.
At some point we should probably document that using "= {}" or "= { 0 }"
in the kernel is a really bad idea and should be avoided.
Moreover, it seems like different compilers seem to behave relatively
differently with these and we often get reports of warnings with these
on clang. When in doubt, memset.
There are quite a few of those under drivers/gpu/drm, for "amd/", "scheduler/"
drm*.c files,
$find . \( -regex "./drm.*\.c" -or -regex "./amd/.*\.c" -or -regex "./scheduler/.*\.c" \) -exec egrep -n -- " *= *{ *(|NULL|0) *}" \{\} \+ | wc -l
374
$_
Out of which only 16 are of the non-ISO C variety, "= {}",
$find . \( -regex "./drm.*\.c" -or -regex "./amd/.*\.c" -or -regex "./scheduler/.*\.c" \) -exec egrep -n -- " *= *{ *}" \{\} \+ | wc -l
16
$_
Perhaps the latter are the more pressing ones, since it is a C++ initializer and not a ISO C one.
It only matters when we care copying the data to userspace, if it all
stays in the kernel, all is fine.
Well only as long as you don't try to compute a CRC32, MD5 or any
fingerprint for a hash from the bytes from the structure.
Then it fails horrible and you wonder why the code doesn't works as
expected.
Regards,
Christian.
thanks,
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
