Hello Jack Xiao,
The patch d0c423b64765: "drm/amdgpu/mes: use ring for kernel queue
submission" from Mar 27, 2020, leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1056 amdgpu_mes_add_ring()
error: format string overflow. buf_size: 16 length: 38 [user data]
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
980 int amdgpu_mes_add_ring(struct amdgpu_device *adev, int gang_id,
981 int queue_type, int idx,
982 struct amdgpu_mes_ctx_data *ctx_data,
983 struct amdgpu_ring **out)
984 {
985 struct amdgpu_ring *ring;
986 struct amdgpu_mes_gang *gang;
987 struct amdgpu_mes_queue_properties qprops = {0};
988 int r, queue_id, pasid;
989
990 /*
991 * Avoid taking any other locks under MES lock to avoid circular
992 * lock dependencies.
993 */
994 amdgpu_mes_lock(&adev->mes);
995 gang = idr_find(&adev->mes.gang_id_idr, gang_id);
996 if (!gang) {
997 DRM_ERROR("gang id %d doesn't exist\n", gang_id);
998 amdgpu_mes_unlock(&adev->mes);
999 return -EINVAL;
1000 }
1001 pasid = gang->process->pasid;
1002
1003 ring = kzalloc(sizeof(struct amdgpu_ring), GFP_KERNEL);
1004 if (!ring) {
1005 amdgpu_mes_unlock(&adev->mes);
1006 return -ENOMEM;
1007 }
1008
1009 ring->ring_obj = NULL;
1010 ring->use_doorbell = true;
1011 ring->is_mes_queue = true;
1012 ring->mes_ctx = ctx_data;
1013 ring->idx = idx;
1014 ring->no_scheduler = true;
1015
1016 if (queue_type == AMDGPU_RING_TYPE_COMPUTE) {
1017 int offset = offsetof(struct amdgpu_mes_ctx_meta_data,
1018 compute[ring->idx].mec_hpd);
1019 ring->eop_gpu_addr =
1020 amdgpu_mes_ctx_get_offs_gpu_addr(ring, offset);
1021 }
1022
1023 switch (queue_type) {
1024 case AMDGPU_RING_TYPE_GFX:
1025 ring->funcs = adev->gfx.gfx_ring[0].funcs;
1026 break;
1027 case AMDGPU_RING_TYPE_COMPUTE:
1028 ring->funcs = adev->gfx.compute_ring[0].funcs;
1029 break;
1030 case AMDGPU_RING_TYPE_SDMA:
1031 ring->funcs = adev->sdma.instance[0].ring.funcs;
1032 break;
1033 default:
1034 BUG();
1035 }
1036
1037 r = amdgpu_ring_init(adev, ring, 1024, NULL, 0,
1038 AMDGPU_RING_PRIO_DEFAULT, NULL);
1039 if (r)
1040 goto clean_up_memory;
1041
1042 amdgpu_mes_ring_to_queue_props(adev, ring, &qprops);
1043
1044 dma_fence_wait(gang->process->vm->last_update, false);
1045 dma_fence_wait(ctx_data->meta_data_va->last_pt_update, false);
1046 amdgpu_mes_unlock(&adev->mes);
1047
1048 r = amdgpu_mes_add_hw_queue(adev, gang_id, &qprops, &queue_id);
1049 if (r)
1050 goto clean_up_ring;
1051
1052 ring->hw_queue_id = queue_id;
1053 ring->doorbell_index = qprops.doorbell_off;
1054
1055 if (queue_type == AMDGPU_RING_TYPE_GFX)
--> 1056 sprintf(ring->name, "gfx_%d.%d.%d", pasid, gang_id,
queue_id);
I'm not sure why this is warning now instead of in 2020. But the bug is
definitely real. "gang_id" is capped at INT_MAX so that can overflow
already even if the values of "pasid" and "queue_id" are zero.
Using snprintf() is safer but also probably the buffer should be larger.
1057 else if (queue_type == AMDGPU_RING_TYPE_COMPUTE)
1058 sprintf(ring->name, "compute_%d.%d.%d", pasid, gang_id,
1059 queue_id);
1060 else if (queue_type == AMDGPU_RING_TYPE_SDMA)
1061 sprintf(ring->name, "sdma_%d.%d.%d", pasid, gang_id,
1062 queue_id);
1063 else
1064 BUG();
1065
1066 *out = ring;
1067 return 0;
1068
1069 clean_up_ring:
1070 amdgpu_ring_fini(ring);
1071 clean_up_memory:
1072 kfree(ring);
1073 amdgpu_mes_unlock(&adev->mes);
1074 return r;
1075 }
regards,
dan carpenter
