Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1644945

[uwb-i1480] question about value overwrite

Show all headers | View raw

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.

Thank you!
--
Gustavo A. R. Silva

Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread

Thread

[uwb-i1480] question about value overwrite "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-05-19 01:10 +0200
  Re: [uwb-i1480] question about value overwrite Greg KH <gregkh@linuxfoundation.org> - 2017-05-19 07:40 +0200
    Re: [uwb-i1480] question about value overwrite "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-05-19 10:20 +0200
      [PATCH] uwb: i1480: add missing goto "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-05-19 10:30 +0200

csiph-web