Re: [PATCH v13] can: virtio: Add virtio CAN driver

[Dorinda Bassey <dbassey@redhat.com>]

Hi Matias,

I've been testing PATCH v13 of the virtio CAN driver and encountered a
FORTIFY_SOURCE panic when transmitting frames:

	sh-5.3# cansend can0 123#DEADBEEF                                                                                     
	[   51.700501] Kernel BUG at __fortify_panic+0x9/0xb [verbose debug info unavailable]                                 
	[   51.700798] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI                                                      
	[   51.700881] CPU: 2 UID: 0 PID: 374 Comm: cansend Tainted: G        W          6.12.76 #1                           
	[   51.701070] Tainted: [W]=WARN                                                                                      
	[   51.701143] RIP: 0010:__fortify_panic+0x9/0xb                                                                      
	[   51.701212] Code: 01 00 00 e9 58 7e c2 ff cc cc cc cc cc cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90  
	90 40 0f b6 ff e8 57 a9 c2 ff <0f> 0b 48 8b 54 24 08 48 8b 74 24 10 4c 8d 44 24 1d 4c 89 e1 48 c7                     
	[   51.701406] RSP: 0018:ffffc900001ffb10 EFLAGS: 00010246                                                            
	[   51.701454] RAX: 0000000000000000 RBX: ffff888100ea8780 RCX: 0000000000000003                                      
	[   51.701530] RDX: 0000000000000000 RSI: ffffc900001ff9b8 RDI: 0000000000000001                                      
	[   51.701625] RBP: 0000000000000000 R08: 0000000000000000 R09: 00000000fffffbff                                      
	[   51.701700] R10: ffffffff82239ee0 R11: ffffc900001ff9b0 R12: ffff888100ea8000                                      
	[   51.701789] R13: ffff888100817200 R14: ffff88810037cda0 R15: ffffc900001ffb48                                      
	[   51.701866] FS:  00007f7c4cda3740(0000) GS:ffff88812bd00000(0000) knlGS:0000000000000000                           
	[   51.701948] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033                                                      
	[   51.702007] CR2: 00007f7c4ceffdc0 CR3: 0000000100d12000 CR4: 0000000000350eb0                                      
	[   51.702072] Call Trace:                                                                                            
	[   51.702105]  <TASK>                                                                                                
	[   51.702126]  ? virtio_can_start_xmit.cold+0x2b/0x4d                                                                
	[   51.702171]  ? srso_alias_return_thunk+0x5/0xfbef5 

The issue is in virtio_can_start_xmit() where can_tx_msg->tx_out.length
is set AFTER memcpy(can_tx_msg->tx_out.sdu, ...). Since sdu[] uses
__counted_by_le(length), FORTIFY_SOURCE sees length=0 during the copy
and panics.

The fix is to set length before the memcpy:

diff --git a/drivers/net/can/virtio_can.c b/drivers/net/can/virtio_can.c
index xxx..yyy 100644
--- a/drivers/net/can/virtio_can.c
+++ b/drivers/net/can/virtio_can.c
@@ -308,6 +308,7 @@ static netdev_tx_t virtio_can_start_xmit(struct sk_buff *skb,
 
 	can_tx_msg->tx_out.msg_type = cpu_to_le16(VIRTIO_CAN_TX);
+	can_tx_msg->tx_out.length = cpu_to_le16(cf->len);
 	can_flags = 0;
 
 	if (cf->can_id & CAN_EFF_FLAG) {
@@ -322,7 +323,6 @@ static netdev_tx_t virtio_can_start_xmit(struct sk_buff *skb,
 		can_flags |= VIRTIO_CAN_FLAGS_FD;
 
 	can_tx_msg->tx_out.flags = cpu_to_le32(can_flags);
-	can_tx_msg->tx_out.length = cpu_to_le16(cf->len);
 
 	sg_init_one(&sg_out, &can_tx_msg->tx_out, hdr_size + cf->len);

Tested with vhost-device-can backend, and it works correctly after this fix.

Thanks,
Dorinda Bassey