Re: [bug report] drm/amdgpu: add amdgpu_ras.c to support ras (v2)

[Dan Carpenter <dan.carpenter@oracle.com>]

On Wed, May 06, 2020 at 07:26:16AM +0000, Zhou1, Tao wrote:
> [AMD Public Use]
> 
> Hi Dan:
> 
> Please check the following piece of code in amdgpu_ras_debugfs_ctrl_parse_data:
> 
> 	if (op != -1) {
> 		if (amdgpu_ras_find_block_id_by_name(block_name, &block_id))
> 			return -EINVAL;
> 
> 		data->head.block = block_id;
> 
> amdgpu_ras_find_block_id_by_name will return error directly if someone try to provide an invalid block_name intentionally via debugfs.
> 

No.  It's the line after that which are the problem.

drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
   147  static int amdgpu_ras_debugfs_ctrl_parse_data(struct file *f,
   148                  const char __user *buf, size_t size,
   149                  loff_t *pos, struct ras_debug_if *data)
   150  {
   151          ssize_t s = min_t(u64, 64, size);
   152          char str[65];
   153          char block_name[33];
   154          char err[9] = "ue";
   155          int op = -1;
   156          int block_id;
   157          uint32_t sub_block;
   158          u64 address, value;
   159  
   160          if (*pos)
   161                  return -EINVAL;
   162          *pos = size;
   163  
   164          memset(str, 0, sizeof(str));
   165          memset(data, 0, sizeof(*data));
   166  
   167          if (copy_from_user(str, buf, s))
   168                  return -EINVAL;
   169  
   170          if (sscanf(str, "disable %32s", block_name) == 1)
   171                  op = 0;
   172          else if (sscanf(str, "enable %32s %8s", block_name, err) == 2)
   173                  op = 1;
   174          else if (sscanf(str, "inject %32s %8s", block_name, err) == 2)
   175                  op = 2;
   176          else if (str[0] && str[1] && str[2] && str[3])
   177                  /* ascii string, but commands are not matched. */

Say we don't write an ascii string.

   178                  return -EINVAL;
   179  
   180          if (op != -1) {
   181                  if (amdgpu_ras_find_block_id_by_name(block_name, &block_id))
   182                          return -EINVAL;
   183  
   184                  data->head.block = block_id;
   185                  /* only ue and ce errors are supported */
   186                  if (!memcmp("ue", err, 2))
   187                          data->head.type = AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
   188                  else if (!memcmp("ce", err, 2))
   189                          data->head.type = AMDGPU_RAS_ERROR__SINGLE_CORRECTABLE;
   190                  else
   191                          return -EINVAL;
   192  
   193                  data->op = op;
   194  
   195                  if (op == 2) {
   196                          if (sscanf(str, "%*s %*s %*s %u %llu %llu",
   197                                                  &sub_block, &address, &value) != 3)
   198                                  if (sscanf(str, "%*s %*s %*s 0x%x 0x%llx 0x%llx",
   199                                                          &sub_block, &address, &value) != 3)
   200                                          return -EINVAL;
   201                          data->head.sub_block_index = sub_block;
   202                          data->inject.address = address;
   203                          data->inject.value = value;
   204                  }
   205          } else {
   206                  if (size < sizeof(*data))
   207                          return -EINVAL;
   208  
   209                  if (copy_from_user(data, buf, sizeof(*data)))
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This lets us set the data->head.block to whatever we want.  Premusably
there is a trusted app which knows how to write the correct values.
But if it has a bug that will cause a crash and we'll have to find a
way to disable it in the kernel for kernel lock down mode etc so either
way we'll need to do a bit of work.

   210                          return -EINVAL;
   211          }
   212  
   213          return 0;
   214  }

regards,
dan carpenter

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