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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.