[ Potential security issue, if I'm reading the code correctly. I don't really know the code and I haven't looked at the larger context. -dan ]
Hello Leo (Sunpeng) Li,
The patch edf6ffe4f47e: "drm/amd/display: Read AUX channel even if only status byte is returned" from Jun 26, 2018, leads to the following static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_ddc.c:673 dc_link_aux_transfer() warn: 'returned_bytes' is unsigned
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_ddc.c 634 int dc_link_aux_transfer(struct ddc_service *ddc, 635 unsigned int address, 636 uint8_t *reply, 637 void *buffer, 638 unsigned int size, 639 enum aux_transaction_type type, 640 enum i2caux_transaction_action action) 641 { 642 struct ddc *ddc_pin = ddc->ddc_pin; 643 struct engine *engine; 644 struct aux_engine *aux_engine; 645 enum aux_channel_operation_result operation_result; 646 struct aux_request_transaction_data aux_req; 647 struct aux_reply_transaction_data aux_rep; 648 uint8_t returned_bytes = 0; ^^^^^^^^^^^^^^^^^^^^^^^^^^ returned_bytes is a u8.
649 int res = -1; 650 uint32_t status; 651 652 memset(&aux_req, 0, sizeof(aux_req)); 653 memset(&aux_rep, 0, sizeof(aux_rep)); 654 655 engine = ddc->ctx->dc->res_pool->engines[ddc_pin->pin_data->en]; 656 aux_engine = engine->funcs->acquire(engine, ddc_pin); 657 658 aux_req.type = type; 659 aux_req.action = action; 660 661 aux_req.address = address; 662 aux_req.delay = 0; 663 aux_req.length = size; 664 aux_req.data = buffer; 665 666 aux_engine->funcs->submit_channel_request(aux_engine, &aux_req); 667 operation_result = aux_engine->funcs->get_channel_status(aux_engine, &returned_bytes); 668 669 switch (operation_result) { 670 case AUX_CHANNEL_OPERATION_SUCCEEDED: 671 res = returned_bytes; ^^^^^^^^^^^^^^^^^^^^ res = 0-255
672 673 if (res <= size && res >= 0) ^^^^^^^^^^^^^^^^^^^^^^^ So obviously the res >= 0 check can be removed, but that's harmless. The bigger problem is that the other test looks to be reversed. Instead of <= it should be >= size. Otherwise we are reading beyond the end of the returned bytes.
674 res = aux_engine->funcs->read_channel_reply(aux_engine, size, 675 buffer, reply, 676 &status); 677 678 break;
regards, dan carpenter