[PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[Oliver Hartkopp]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

from Oliver Hartkopp <oliver@hartkopp.net>

The tx packet counting and the local loopback of CAN frames should
only happen in the case that the CAN frame has been enqueued to the
netdevice tx queue successfully.

Thanks to Andre Naujoks <nautsch@gmail.com> for reporting this issue.

Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>

---

Hello Dave,

this patch should go into 2.6.26 and also applies fine on 2.6.25.1.

IMO this issue is not that 'critical' that it *requires* to become
part of 2.6.25.2. Please forward the patch to Greg at your own opinion.

Many Thanks!
Oliver




[-- Attachment #2: can_send.diff --]
[-- Type: text/x-diff, Size: 1312 bytes --]

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 2759b76..7e8ca28 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -208,6 +208,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol)
  */
 int can_send(struct sk_buff *skb, int loop)
 {
+	struct sk_buff *newskb = NULL;
 	int err;
 
 	if (skb->dev->type != ARPHRD_CAN) {
@@ -244,8 +245,7 @@ int can_send(struct sk_buff *skb, int loop)
 			 * If the interface is not capable to do loopback
 			 * itself, we do it here.
 			 */
-			struct sk_buff *newskb = skb_clone(skb, GFP_ATOMIC);
-
+			newskb = skb_clone(skb, GFP_ATOMIC);
 			if (!newskb) {
 				kfree_skb(skb);
 				return -ENOMEM;
@@ -254,7 +254,6 @@ int can_send(struct sk_buff *skb, int loop)
 			newskb->sk = skb->sk;
 			newskb->ip_summed = CHECKSUM_UNNECESSARY;
 			newskb->pkt_type = PACKET_BROADCAST;
-			netif_rx(newskb);
 		}
 	} else {
 		/* indication for the CAN driver: no loopback required */
@@ -266,11 +265,20 @@ int can_send(struct sk_buff *skb, int loop)
 	if (err > 0)
 		err = net_xmit_errno(err);
 
+	if (err) {
+		if (newskb)
+			kfree_skb(newskb);
+		return err;
+	}
+
+	if (newskb)
+		netif_rx(newskb);
+
 	/* update statistics */
 	can_stats.tx_frames++;
 	can_stats.tx_frames_delta++;
 
-	return err;
+	return 0;
 }
 EXPORT_SYMBOL(can_send);
 

Re: [PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[David Miller]

From: Oliver Hartkopp <oliver@hartkopp.net>
Date: Tue, 06 May 2008 21:49:57 +0200

> The tx packet counting and the local loopback of CAN frames should
> only happen in the case that the CAN frame has been enqueued to the
> netdevice tx queue successfully.
> 
> Thanks to Andre Naujoks <nautsch@gmail.com> for reporting this issue.
> 
> Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
> Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>

Applied, thanks Oliver.

> this patch should go into 2.6.26 and also applies fine on 2.6.25.1.
> 
> IMO this issue is not that 'critical' that it *requires* to become
> part of 2.6.25.2. Please forward the patch to Greg at your own opinion.

I think I'll push it to -stable, thanks.

Re: [PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[Oliver Hartkopp]

David Miller wrote:
>
>> this patch should go into 2.6.26 and also applies fine on 2.6.25.1.
>>
>> IMO this issue is not that 'critical' that it *requires* to become
>> part of 2.6.25.2. Please forward the patch to Greg at your own opinion.
>>     
>
> I think I'll push it to -stable, thanks.
>
>   

Thanks!

Maybe we should also consider to push the patch from Sam regarding the 
wrong copy_from_user() results interpretation to the stable tree:

can: Fix copy_from_user() results interpretation

author 	Sam Ravnborg <sam@ravnborg.org>

	Sun, 27 Apr 2008 05:57:25 +0000 (22:57 -0700)
committer 	David S. Miller <davem@davemloft.net>

	Sun, 27 Apr 2008 21:26:51 +0000 (14:26 -0700)
commit 	3f91bd420a955803421f2db17b2e04aacfbb2bb8



Regards,
Oliver

Re: [PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[Sam Ravnborg]

On Thu, May 08, 2008 at 07:00:39PM +0200, Oliver Hartkopp wrote:
> David Miller wrote:
> >
> >>this patch should go into 2.6.26 and also applies fine on 2.6.25.1.
> >>
> >>IMO this issue is not that 'critical' that it *requires* to become
> >>part of 2.6.25.2. Please forward the patch to Greg at your own opinion.
> >>    
> >
> >I think I'll push it to -stable, thanks.
> >
> >  
> 
> Thanks!
> 
> Maybe we should also consider to push the patch from Sam regarding the 
> wrong copy_from_user() results interpretation to the stable tree:
> 
> can: Fix copy_from_user() results interpretation
> 
> author 	Sam Ravnborg <sam@ravnborg.org>
> 
> 	Sun, 27 Apr 2008 05:57:25 +0000 (22:57 -0700)
> committer 	David S. Miller <davem@davemloft.net>
> 
> 	Sun, 27 Apr 2008 21:26:51 +0000 (14:26 -0700)
> commit 	3f91bd420a955803421f2db17b2e04aacfbb2bb8

I recall tat I considered stable material when I did
this patch. But as I merely just did a codingstyle update
of the old patch I dropped it.

But looks like possible -stable materail so I've added
stable@kernel.org to the to list so they can judge.

	Sam

Re: [PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[Oliver Hartkopp]

Sam Ravnborg wrote:
> On Thu, May 08, 2008 at 07:00:39PM +0200, Oliver Hartkopp wrote:
>   
>>
>>
>> Maybe we should also consider to push the patch from Sam regarding the 
>> wrong copy_from_user() results interpretation to the stable tree:
>>
>> can: Fix copy_from_user() results interpretation
>>
>> author 	Sam Ravnborg <sam@ravnborg.org>
>>
>> 	Sun, 27 Apr 2008 05:57:25 +0000 (22:57 -0700)
>> committer 	David S. Miller <davem@davemloft.net>
>>
>> 	Sun, 27 Apr 2008 21:26:51 +0000 (14:26 -0700)
>> commit 	3f91bd420a955803421f2db17b2e04aacfbb2bb8
>>     
>
> I recall tat I considered stable material when I did
> this patch. But as I merely just did a codingstyle update
> of the old patch I dropped it.
>
> But looks like possible -stable materail so I've added
> stable@kernel.org to the to list so they can judge.
>   

Yep. Thanks.

Indeed you updated the codingstyle from a patch suggested by Pavel - but 
his (original) patch fixes the copy_from_user() result interpretation. 
So it was not only a codingstyle patch which was commited here.

Best regards,
Oliver

Re: [stable] [PATCH] [2.6.26] [CAN] Fix can_send() handling on dev_queue_xmit() failures

[Greg KH]

On Thu, May 08, 2008 at 07:29:38PM +0200, Sam Ravnborg wrote:
> On Thu, May 08, 2008 at 07:00:39PM +0200, Oliver Hartkopp wrote:
> > David Miller wrote:
> > >
> > >>this patch should go into 2.6.26 and also applies fine on 2.6.25.1.
> > >>
> > >>IMO this issue is not that 'critical' that it *requires* to become
> > >>part of 2.6.25.2. Please forward the patch to Greg at your own opinion.
> > >>    
> > >
> > >I think I'll push it to -stable, thanks.
> > >
> > >  
> > 
> > Thanks!
> > 
> > Maybe we should also consider to push the patch from Sam regarding the 
> > wrong copy_from_user() results interpretation to the stable tree:
> > 
> > can: Fix copy_from_user() results interpretation
> > 
> > author 	Sam Ravnborg <sam@ravnborg.org>
> > 
> > 	Sun, 27 Apr 2008 05:57:25 +0000 (22:57 -0700)
> > committer 	David S. Miller <davem@davemloft.net>
> > 
> > 	Sun, 27 Apr 2008 21:26:51 +0000 (14:26 -0700)
> > commit 	3f91bd420a955803421f2db17b2e04aacfbb2bb8
> 
> I recall tat I considered stable material when I did
> this patch. But as I merely just did a codingstyle update
> of the old patch I dropped it.
> 
> But looks like possible -stable materail so I've added
> stable@kernel.org to the to list so they can judge.

I trust David to send the -stable team any networking patches he things
needs to go there.  So it's really up to him.

thanks,

greg k-h