From: Dan Carpenter <dan.carpenter@oracle.com>
To: mkl@pengutronix.de
Cc: linux-can@vger.kernel.org
Subject: [bug report] can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN
Date: Wed, 23 Sep 2020 12:54:47 +0300 [thread overview]
Message-ID: <20200923095447.GA1464378@mwanda> (raw)
Hello Marc Kleine-Budde,
The patch 55e5b97f003e: "can: mcp25xxfd: add driver for Microchip
MCP25xxFD SPI CAN" from Sep 18, 2020, leads to the following static
checker Smatch warning:
drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c:2271 mcp25xxfd_tx_obj_from_skb() warn: user controlled 'len' cast to postive rl = '(-249)-(-1),1-67'
drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c:2272 mcp25xxfd_tx_obj_from_skb() error: 'memcpy()' 'hw_tx_obj->data' too small (64 vs 255)
drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c:2272 mcp25xxfd_tx_obj_from_skb() error: 'memcpy()' 'cfd->data' too small (64 vs 255)
drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c:2272 mcp25xxfd_tx_obj_from_skb() error: 'cfd->len' from user is not capped properly
(Only one of these checks is published and it's disabled unless you
run with the --spammy flag).
drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
2208 static void
2209 mcp25xxfd_tx_obj_from_skb(const struct mcp25xxfd_priv *priv,
2210 struct mcp25xxfd_tx_obj *tx_obj,
2211 const struct sk_buff *skb,
2212 unsigned int seq)
2213 {
2214 const struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Smatch distrusts all skb->data information.
2215 struct mcp25xxfd_hw_tx_obj_raw *hw_tx_obj;
2216 union mcp25xxfd_tx_obj_load_buf *load_buf;
2217 u8 dlc;
2218 u32 id, flags;
2219 int offset, len;
2220
2221 if (cfd->can_id & CAN_EFF_FLAG) {
2222 u32 sid, eid;
2223
2224 sid = FIELD_GET(MCP25XXFD_REG_FRAME_EFF_SID_MASK, cfd->can_id);
2225 eid = FIELD_GET(MCP25XXFD_REG_FRAME_EFF_EID_MASK, cfd->can_id);
2226
2227 id = FIELD_PREP(MCP25XXFD_OBJ_ID_EID_MASK, eid) |
2228 FIELD_PREP(MCP25XXFD_OBJ_ID_SID_MASK, sid);
2229
2230 flags = MCP25XXFD_OBJ_FLAGS_IDE;
2231 } else {
2232 id = FIELD_PREP(MCP25XXFD_OBJ_ID_SID_MASK, cfd->can_id);
2233 flags = 0;
2234 }
2235
2236 /* Use the MCP2518FD mask even on the MCP2517FD. It doesn't
2237 * harm, only the lower 7 bits will be transferred into the
2238 * TEF object.
2239 */
2240 dlc = can_len2dlc(cfd->len);
2241 flags |= FIELD_PREP(MCP25XXFD_OBJ_FLAGS_SEQ_MCP2518FD_MASK, seq) |
2242 FIELD_PREP(MCP25XXFD_OBJ_FLAGS_DLC, dlc);
2243
2244 if (cfd->can_id & CAN_RTR_FLAG)
2245 flags |= MCP25XXFD_OBJ_FLAGS_RTR;
2246
2247 /* CANFD */
2248 if (can_is_canfd_skb(skb)) {
2249 if (cfd->flags & CANFD_ESI)
2250 flags |= MCP25XXFD_OBJ_FLAGS_ESI;
2251
2252 flags |= MCP25XXFD_OBJ_FLAGS_FDF;
2253
2254 if (cfd->flags & CANFD_BRS)
2255 flags |= MCP25XXFD_OBJ_FLAGS_BRS;
2256 }
2257
2258 load_buf = &tx_obj->buf;
2259 if (priv->devtype_data.quirks & MCP25XXFD_QUIRK_CRC_TX)
2260 hw_tx_obj = &load_buf->crc.hw_tx_obj;
2261 else
2262 hw_tx_obj = &load_buf->nocrc.hw_tx_obj;
2263
2264 put_unaligned_le32(id, &hw_tx_obj->id);
2265 put_unaligned_le32(flags, &hw_tx_obj->flags);
2266
2267 /* Clear data at end of CAN frame */
2268 offset = round_down(cfd->len, sizeof(u32));
2269 len = round_up(can_dlc2len(dlc), sizeof(u32)) - offset;
Smatch thinks that "cfd->len" can be more than 64 which leads to setting
"len" negative.
2270 if (MCP25XXFD_SANITIZE_CAN && len)
2271 memset(hw_tx_obj->data + offset, 0x0, len);
^^^
Which would corrupt memory.
2272 memcpy(hw_tx_obj->data, cfd->data, cfd->len);
^^^^^^^^
Smatch thinks this can be more than 64.
2273
2274 /* Number of bytes to be written into the RAM of the controller */
2275 len = sizeof(hw_tx_obj->id) + sizeof(hw_tx_obj->flags);
regards,
dan carpenter
next reply other threads:[~2020-09-23 9:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-23 9:54 Dan Carpenter [this message]
2020-09-23 10:26 ` [bug report] can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN Marc Kleine-Budde
2020-09-23 10:38 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2020-09-23 11:29 Dan Carpenter
2020-09-23 11:43 ` Marc Kleine-Budde
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200923095447.GA1464378@mwanda \
--to=dan.carpenter@oracle.com \
--cc=linux-can@vger.kernel.org \
--cc=mkl@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox