| Message ID | 20201008061821.24663-1-xiyou.wangcong@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net] can: initialize skbcnt in j1939_tp_tx_dat_new() | expand |
On Wed, 7 Oct 2020 23:18:21 -0700 Cong Wang wrote: > This fixes an uninit-value warning: > BUG: KMSAN: uninit-value in can_receive+0x26b/0x630 net/can/af_can.c:650 > > Reported-and-tested-by: syzbot+3f3837e61a48d32b495f@syzkaller.appspotmail.com > Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") > Cc: Robin van der Gracht <robin@protonic.nl> > Cc: Oleksij Rempel <linux@rempel-privat.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Oliver Hartkopp <socketcan@hartkopp.net> > Cc: Marc Kleine-Budde <mkl@pengutronix.de> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > --- > net/can/j1939/transport.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c > index 0cec4152f979..88cf1062e1e9 100644 > --- a/net/can/j1939/transport.c > +++ b/net/can/j1939/transport.c > @@ -580,6 +580,7 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv, > skb->dev = priv->ndev; > can_skb_reserve(skb); > can_skb_prv(skb)->ifindex = priv->ndev->ifindex; > + can_skb_prv(skb)->skbcnt = 0; > /* reserve CAN header */ > skb_reserve(skb, offsetof(struct can_frame, data)); Thanks! Looks like there is another can_skb_reserve(skb) on line 1489, is that one fine? Marc - should I take this directly into net, in case there is a last minute PR to Linus for 5.9?
On Thu, Oct 8, 2020 at 10:34 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 7 Oct 2020 23:18:21 -0700 Cong Wang wrote: > > This fixes an uninit-value warning: > > BUG: KMSAN: uninit-value in can_receive+0x26b/0x630 net/can/af_can.c:650 > > > > Reported-and-tested-by: syzbot+3f3837e61a48d32b495f@syzkaller.appspotmail.com > > Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") > > Cc: Robin van der Gracht <robin@protonic.nl> > > Cc: Oleksij Rempel <linux@rempel-privat.de> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > > Cc: Oliver Hartkopp <socketcan@hartkopp.net> > > Cc: Marc Kleine-Budde <mkl@pengutronix.de> > > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > > --- > > net/can/j1939/transport.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c > > index 0cec4152f979..88cf1062e1e9 100644 > > --- a/net/can/j1939/transport.c > > +++ b/net/can/j1939/transport.c > > @@ -580,6 +580,7 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv, > > skb->dev = priv->ndev; > > can_skb_reserve(skb); > > can_skb_prv(skb)->ifindex = priv->ndev->ifindex; > > + can_skb_prv(skb)->skbcnt = 0; > > /* reserve CAN header */ > > skb_reserve(skb, offsetof(struct can_frame, data)); > > Thanks! Looks like there is another can_skb_reserve(skb) on line 1489, > is that one fine? I don't know, I only attempt to address the syzbot report. To me, it at least does not harm to fix that one too. I am fine either way. Thanks.
On 10/8/20 7:34 PM, Jakub Kicinski wrote: > On Wed, 7 Oct 2020 23:18:21 -0700 Cong Wang wrote: >> This fixes an uninit-value warning: >> BUG: KMSAN: uninit-value in can_receive+0x26b/0x630 net/can/af_can.c:650 >> >> Reported-and-tested-by: syzbot+3f3837e61a48d32b495f@syzkaller.appspotmail.com >> Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") >> Cc: Robin van der Gracht <robin@protonic.nl> >> Cc: Oleksij Rempel <linux@rempel-privat.de> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> >> Cc: Oliver Hartkopp <socketcan@hartkopp.net> >> Cc: Marc Kleine-Budde <mkl@pengutronix.de> >> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> >> --- >> net/can/j1939/transport.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c >> index 0cec4152f979..88cf1062e1e9 100644 >> --- a/net/can/j1939/transport.c >> +++ b/net/can/j1939/transport.c >> @@ -580,6 +580,7 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv, >> skb->dev = priv->ndev; >> can_skb_reserve(skb); >> can_skb_prv(skb)->ifindex = priv->ndev->ifindex; >> + can_skb_prv(skb)->skbcnt = 0; >> /* reserve CAN header */ >> skb_reserve(skb, offsetof(struct can_frame, data)); > > Thanks! Looks like there is another can_skb_reserve(skb) on line 1489, > is that one fine? I think say yes, but better safe the sorry. > Marc - should I take this directly into net, in case there is a last > minute PR to Linus for 5.9? Yes, of you can pull Cong Wang's patch and my patch, that fixes the other missing init of skbcnt. That tag includes my previous oneline-patch-pull-request from 20201006, that fixes the c_can driver on basically all stm32mp1 based boards. Would be good to see that in 5.9, as well. git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-5.9-20201008 I've send a proper pull request some seconds ago. Marc
On Thu, 8 Oct 2020 23:47:38 +0200 Marc Kleine-Budde wrote: > > Marc - should I take this directly into net, in case there is a last > > minute PR to Linus for 5.9? > > Yes, of you can pull Cong Wang's patch and my patch, that fixes the other > missing init of skbcnt. > > That tag includes my previous oneline-patch-pull-request from 20201006, that > fixes the c_can driver on basically all stm32mp1 based boards. Would be good to > see that in 5.9, as well. > > git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git > tags/linux-can-fixes-for-5.9-20201008 > > I've send a proper pull request some seconds ago. Sorry for the rush but I didn't want to wait too much, since we're already at -rc8. I already sent the PR to Linus. I'll pull from you and queue the fixes up for stable, hope that works.
On 10/9/20 12:26 AM, Jakub Kicinski wrote: > On Thu, 8 Oct 2020 23:47:38 +0200 Marc Kleine-Budde wrote: >>> Marc - should I take this directly into net, in case there is a last >>> minute PR to Linus for 5.9? >> >> Yes, of you can pull Cong Wang's patch and my patch, that fixes the other >> missing init of skbcnt. >> >> That tag includes my previous oneline-patch-pull-request from 20201006, that >> fixes the c_can driver on basically all stm32mp1 based boards. Would be good to >> see that in 5.9, as well. >> >> git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git >> tags/linux-can-fixes-for-5.9-20201008 >> >> I've send a proper pull request some seconds ago. > > Sorry for the rush but I didn't want to wait too much, since we're > already at -rc8. I already sent the PR to Linus. I'll pull from you > and queue the fixes up for stable, hope that works. Fine with me. regards, Marc
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 0cec4152f979..88cf1062e1e9 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -580,6 +580,7 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv, skb->dev = priv->ndev; can_skb_reserve(skb); can_skb_prv(skb)->ifindex = priv->ndev->ifindex; + can_skb_prv(skb)->skbcnt = 0; /* reserve CAN header */ skb_reserve(skb, offsetof(struct can_frame, data));
This fixes an uninit-value warning: BUG: KMSAN: uninit-value in can_receive+0x26b/0x630 net/can/af_can.c:650 Reported-and-tested-by: syzbot+3f3837e61a48d32b495f@syzkaller.appspotmail.com Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Cc: Robin van der Gracht <robin@protonic.nl> Cc: Oleksij Rempel <linux@rempel-privat.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Oliver Hartkopp <socketcan@hartkopp.net> Cc: Marc Kleine-Budde <mkl@pengutronix.de> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> --- net/can/j1939/transport.c | 1 + 1 file changed, 1 insertion(+)