Hi Greg,

Quoting Greg KH <gre...@linuxfoundation.org>:

On Thu, May 18, 2017 at 06:00:06PM -0500, Gustavo A. R. Silva wrote:

Hello everybody,

While looking into Coverity ID 1226913 I ran into the following piece of
code at drivers/uwb/i1480/dfu/phy.c:99:

 99static
100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr, size_t size)
101{
102        int result;
103        struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf;
104        struct i1480_evt_mpi_read *reply = i1480->evt_buf;
105        unsigned cnt;
106
107        memset(i1480->cmd_buf, 0x69, 512);
108        memset(i1480->evt_buf, 0x69, 512);
109
110        BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3);
111        result = -ENOMEM;
112        cmd->rccb.bCommandType = i1480_CET_VS1;
113        cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ);
114        cmd->size = cpu_to_le16(3*size);
115        for (cnt = 0; cnt < size; cnt++) {
116                cmd->data[cnt].page = (srcaddr + cnt) >> 8;
117                cmd->data[cnt].offset = (srcaddr + cnt) & 0xff;
118        }
119        reply->rceb.bEventType = i1480_CET_VS1;
120        reply->rceb.wEvent = i1480_CMD_MPI_READ;
121        result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size,
122                        sizeof(*reply) + 3*size);
123        if (result < 0)
124                goto out;
125        if (reply->bResultCode != UWB_RC_RES_SUCCESS) {
126                dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
127                        reply->bResultCode);
128                result = -EIO;
129        }
130        for (cnt = 0; cnt < size; cnt++) {
131                if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
132                        dev_err(i1480->dev, "MPI-READ: page inconsistency
at "
133                                "index %u: expected 0x%02x, got
0x%02x\n", cnt,
134                                (srcaddr + cnt) >> 8,
reply->data[cnt].page);
135 if (reply->data[cnt].offset != ((srcaddr + cnt) & 0x00ff))
136                        dev_err(i1480->dev, "MPI-READ: offset
inconsistency at "
137                                "index %u: expected 0x%02x, got
0x%02x\n", cnt,
138                                (srcaddr + cnt) & 0x00ff,
139                                reply->data[cnt].offset);
140                data[cnt] = reply->data[cnt].value;
141        }
142        result = 0;
143out:
144        return result;
145}

The issue is that the value store in variable _result_ at line 128 is
overwritten by the one stored at line 142, before it can be used.

My question is if the original intention was to return this value
inmediately after the assignment at line 128, something like in the
following patch:

index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16
srcaddr, size_t size)
                dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
                        reply->bResultCode);
                result = -EIO;
+               goto out;
        }
        for (cnt = 0; cnt < size; cnt++) {
                if (reply->data[cnt].page != (srcaddr + cnt) >> 8)

What do you think?

I'd really appreciate any comment on this.

I think you are correct, I'll take a patch to fix this up if you want to
write one :)


Absolutely, I'll send it shortly.

Thanks!
--
Gustavo A. R. Silva






Reply via email to