diff mbox series

[net] can: initialize skbcnt in j1939_tp_tx_dat_new()

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

Commit Message

Cong Wang Oct. 8, 2020, 6:18 a.m. UTC
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(+)

Comments

Jakub Kicinski Oct. 8, 2020, 5:34 p.m. UTC | #1
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?
Cong Wang Oct. 8, 2020, 8:02 p.m. UTC | #2
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.
Marc Kleine-Budde Oct. 8, 2020, 9:47 p.m. UTC | #3
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
Jakub Kicinski Oct. 8, 2020, 10:26 p.m. UTC | #4
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.
Marc Kleine-Budde Oct. 9, 2020, 6:15 a.m. UTC | #5
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 mbox series

Patch

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));