[bug report] drm/amd/display: Read AUX channel even if only status byte is returned

[Dan Carpenter]

[ 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