Re: [Linux-kernel-mentees] [PATCH] drm/amdgpu: Prevent kernel-infoleak in amdgpu_info_ioctl()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Thanks,
Christian.

---
$ pahole -C "drm_amdgpu_info_device" drivers/gpu/drm/amd/amdgpu/amdgpu_kms.o
struct drm_amdgpu_info_device {
	__u32                      device_id;            /*     0     4 */
	__u32                      chip_rev;             /*     4     4 */
	__u32                      external_rev;         /*     8     4 */
	__u32                      pci_rev;              /*    12     4 */
	__u32                      family;               /*    16     4 */
	__u32                      num_shader_engines;   /*    20     4 */
	__u32                      num_shader_arrays_per_engine; /*    24     4 */
	__u32                      gpu_counter_freq;     /*    28     4 */
	__u64                      max_engine_clock;     /*    32     8 */
	__u64                      max_memory_clock;     /*    40     8 */
	__u32                      cu_active_number;     /*    48     4 */
	__u32                      cu_ao_mask;           /*    52     4 */
	__u32                      cu_bitmap[4][4];      /*    56    64 */
	/* --- cacheline 1 boundary (64 bytes) was 56 bytes ago --- */
	__u32                      enabled_rb_pipes_mask; /*   120     4 */
	__u32                      num_rb_pipes;         /*   124     4 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	__u32                      num_hw_gfx_contexts;  /*   128     4 */
	__u32                      _pad;                 /*   132     4 */
	__u64                      ids_flags;            /*   136     8 */
	__u64                      virtual_address_offset; /*   144     8 */
	__u64                      virtual_address_max;  /*   152     8 */
	__u32                      virtual_address_alignment; /*   160     4 */
	__u32                      pte_fragment_size;    /*   164     4 */
	__u32                      gart_page_size;       /*   168     4 */
	__u32                      ce_ram_size;          /*   172     4 */
	__u32                      vram_type;            /*   176     4 */
	__u32                      vram_bit_width;       /*   180     4 */
	__u32                      vce_harvest_config;   /*   184     4 */
	__u32                      gc_double_offchip_lds_buf; /*   188     4 */
	/* --- cacheline 3 boundary (192 bytes) --- */
	__u64                      prim_buf_gpu_addr;    /*   192     8 */
	__u64                      pos_buf_gpu_addr;     /*   200     8 */
	__u64                      cntl_sb_buf_gpu_addr; /*   208     8 */
	__u64                      param_buf_gpu_addr;   /*   216     8 */
	__u32                      prim_buf_size;        /*   224     4 */
	__u32                      pos_buf_size;         /*   228     4 */
	__u32                      cntl_sb_buf_size;     /*   232     4 */
	__u32                      param_buf_size;       /*   236     4 */
	__u32                      wave_front_size;      /*   240     4 */
	__u32                      num_shader_visible_vgprs; /*   244     4 */
	__u32                      num_cu_per_sh;        /*   248     4 */
	__u32                      num_tcc_blocks;       /*   252     4 */
	/* --- cacheline 4 boundary (256 bytes) --- */
	__u32                      gs_vgt_table_depth;   /*   256     4 */
	__u32                      gs_prim_buffer_depth; /*   260     4 */
	__u32                      max_gs_waves_per_vgt; /*   264     4 */
	__u32                      _pad1;                /*   268     4 */
	__u32                      cu_ao_bitmap[4][4];   /*   272    64 */
	/* --- cacheline 5 boundary (320 bytes) was 16 bytes ago --- */
	__u64                      high_va_offset;       /*   336     8 */
	__u64                      high_va_max;          /*   344     8 */
	__u32                      pa_sc_tile_steering_override; /*   352     4 */

	/* XXX 4 bytes hole, try to pack */

	__u64                      tcc_disabled_mask;    /*   360     8 */

	/* size: 368, cachelines: 6, members: 49 */
	/* sum members: 364, holes: 1, sum holes: 4 */
	/* last cacheline: 48 bytes */
};

  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index a8c47aecd342..0047da06041f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -707,9 +707,10 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
  		return n ? -EFAULT : 0;
  	}
  	case AMDGPU_INFO_DEV_INFO: {
-		struct drm_amdgpu_info_device dev_info = {};
+		struct drm_amdgpu_info_device dev_info;
  		uint64_t vm_size;
+ memset(&dev_info, 0, sizeof(dev_info));
  		dev_info.device_id = dev->pdev->device;
  		dev_info.chip_rev = adev->rev_id;
  		dev_info.external_rev = adev->external_rev_id;

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux