netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new()
@ 2020-10-08  6:18 Cong Wang
  2020-10-08 17:34 ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2020-10-08  6:18 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, syzbot+3f3837e61a48d32b495f, Robin van der Gracht,
	Oleksij Rempel, Pengutronix Kernel Team, Oliver Hartkopp,
	Marc Kleine-Budde

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new()
  2020-10-08  6:18 [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new() Cong Wang
@ 2020-10-08 17:34 ` Jakub Kicinski
  2020-10-08 20:02   ` Cong Wang
  2020-10-08 21:47   ` Marc Kleine-Budde
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Kicinski @ 2020-10-08 17:34 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, syzbot+3f3837e61a48d32b495f, Robin van der Gracht,
	Oleksij Rempel, Pengutronix Kernel Team, Oliver Hartkopp,
	Marc Kleine-Budde

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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new()
  2020-10-08 17:34 ` Jakub Kicinski
@ 2020-10-08 20:02   ` Cong Wang
  2020-10-08 21:47   ` Marc Kleine-Budde
  1 sibling, 0 replies; 6+ messages in thread
From: Cong Wang @ 2020-10-08 20:02 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Linux Kernel Network Developers, syzbot+3f3837e61a48d32b495f,
	Robin van der Gracht, Oleksij Rempel, Pengutronix Kernel Team,
	Oliver Hartkopp, Marc Kleine-Budde

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new()
  2020-10-08 17:34 ` Jakub Kicinski
  2020-10-08 20:02   ` Cong Wang
@ 2020-10-08 21:47   ` Marc Kleine-Budde
  2020-10-08 22:26     ` Jakub Kicinski
  1 sibling, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2020-10-08 21:47 UTC (permalink / raw)
  To: Jakub Kicinski, Cong Wang
  Cc: netdev, syzbot+3f3837e61a48d32b495f, Robin van der Gracht,
	Oleksij Rempel, Oliver Hartkopp, Marc Kleine-Budde


[-- Attachment #1.1: Type: text/plain, Size: 2239 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new()
  2020-10-08 21:47   ` Marc Kleine-Budde
@ 2020-10-08 22:26     ` Jakub Kicinski
  2020-10-09  6:15       ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2020-10-08 22:26 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Cong Wang, netdev, syzbot+3f3837e61a48d32b495f,
	Robin van der Gracht, Oleksij Rempel, Oliver Hartkopp

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new()
  2020-10-08 22:26     ` Jakub Kicinski
@ 2020-10-09  6:15       ` Marc Kleine-Budde
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2020-10-09  6:15 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Cong Wang, netdev, syzbot+3f3837e61a48d32b495f,
	Robin van der Gracht, Oleksij Rempel, Oliver Hartkopp


[-- Attachment #1.1: Type: text/plain, Size: 1205 bytes --]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-09  6:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08  6:18 [Patch net] can: initialize skbcnt in j1939_tp_tx_dat_new() Cong Wang
2020-10-08 17:34 ` Jakub Kicinski
2020-10-08 20:02   ` Cong Wang
2020-10-08 21:47   ` Marc Kleine-Budde
2020-10-08 22:26     ` Jakub Kicinski
2020-10-09  6:15       ` Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).