On Sat, Oct 02, 2021 at 01:59:56AM +0300, Dmitry Baryshkov wrote:
On 01/10/2021 15:36, Dan Carpenter wrote:
The "msg->tx_len" variable is type size_t so if dsi_cmds2buf_tx() returns a negative error code that it type promoted to a high positive value and treat as a success. The second problem with this code is that it can return meaningless positive values on error.
It looks to me that this piece of code is not fully correct at all. dsi_cmds2bus_tx would return the size of DSI packet, not the size of the DSI buffer.
Ugh... I misread what you were saying. I was thinking I could just check for negatives. This sounds like struct_size() thing?
Could you please be more specific, which 'meaningless positive values' were you receiving?
Returning any positive values at this point is a bug. It's supposed to return the number of bytes that were recieved.
And there is another bug as well:
drivers/gpu/drm/msm/dsi/dsi_host.c 1370 static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host, 1371 const struct mipi_dsi_msg *msg) 1372 { 1373 int len, ret; 1374 int bllp_len = msm_host->mode->hdisplay * 1375 dsi_get_bpp(msm_host->format) / 8; 1376 1377 len = dsi_cmd_dma_add(msm_host, msg); 1378 if (!len) {
The dsi_cmd_dma_add() returns negative error codes so this check should be "if (len <= 0) {".
1379 pr_err("%s: failed to add cmd type = 0x%x\n", 1380 __func__, msg->type); 1381 return -EINVAL; 1382 } 1383
I'm not sure about the size of "the DSI packet" Could you handle this one and give me a Reported-by tag? That's probably simpler than another back and forth on email.
regards, dan carpenter