| Message ID | 20210120114137.200019-2-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 03f16c5075b22c8902d2af739969e878b0879c94 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix several use after free bugs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 1/20/21 12:41 PM, Vincent Mailhol wrote: > After calling netif_rx_ni(skb), dereferencing skb is unsafe. > Especially, the can_frame cf which aliases skb memory is accessed > after the netif_rx_ni() in: > stats->rx_bytes += cf->len; > > Reordering the lines solves the issue. > > Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface") > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > --- > *Remark for upstream* > drivers/net/can/dev.c has been moved to drivers/net/can/dev/dev.c in > below commit, please carry the patch forward. > Reference: 3e77f70e7345 ("can: dev: move driver related infrastructure > into separate subdir") I've send a pull request to Jakub and David. Let's see what happens :) Marc
On Wed. 20 janv. 2021 at 21:53, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 1/20/21 12:41 PM, Vincent Mailhol wrote: > > After calling netif_rx_ni(skb), dereferencing skb is unsafe. > > Especially, the can_frame cf which aliases skb memory is accessed > > after the netif_rx_ni() in: > > stats->rx_bytes += cf->len; > > > > Reordering the lines solves the issue. > > > > Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface") > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > --- > > *Remark for upstream* > > drivers/net/can/dev.c has been moved to drivers/net/can/dev/dev.c in > > below commit, please carry the patch forward. > > Reference: 3e77f70e7345 ("can: dev: move driver related infrastructure > > into separate subdir") > > I've send a pull request to Jakub and David. Let's see what happens :) Thanks! Yours sincerely, Vincent > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c index 3486704c8a95..8b1ae023cb21 100644 --- a/drivers/net/can/dev.c +++ b/drivers/net/can/dev.c @@ -592,11 +592,11 @@ static void can_restart(struct net_device *dev) cf->can_id |= CAN_ERR_RESTARTED; - netif_rx_ni(skb); - stats->rx_packets++; stats->rx_bytes += cf->len; + netif_rx_ni(skb); + restart: netdev_dbg(dev, "restarted\n"); priv->can_stats.restarts++;
After calling netif_rx_ni(skb), dereferencing skb is unsafe. Especially, the can_frame cf which aliases skb memory is accessed after the netif_rx_ni() in: stats->rx_bytes += cf->len; Reordering the lines solves the issue. Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface") Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> --- *Remark for upstream* drivers/net/can/dev.c has been moved to drivers/net/can/dev/dev.c in below commit, please carry the patch forward. Reference: 3e77f70e7345 ("can: dev: move driver related infrastructure into separate subdir") --- drivers/net/can/dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)