Re: [RTnet-developers] Bad list operation in af_packet

[Jan K. <jan...@we...>]

On 2011-05-06 17:03, Jesper Christensen wrote:
> Hi
> 
> I have seen a crash in connection with closing an AF_PACKET socket:
> 
> Oops: Kernel access of bad area, sig: 11 [#1]
> PREEMPT SMP NR_CPUS=2 CPCI6200
> Modules linked in: tt_vupgm rtpacket rtudp rtipv4 rt_gfar rt_loopback rtnet
> NIP: b9118ee4 LR: b9188a4c CTR: b9188998
> REGS: ae8edd50 TRAP: 0300   Not tainted  (2.6.29.6)
> MSR: 00021000 <ME,CE>  CR: 22000842  XER: 00000000
> DEAR: 00000000, ESR: 00800000
> TASK = afba6b50[723] 'upgmu' THREAD: ae8ec000 CPU: 0
> GPR00: 00100100 ae8ede00 afba6b50 ae91b3b8 afba6b50 ae8edf50 00000000
> 8056a9f0
> GPR08: 00000001 00000000 00000001 00000000 42000848 1005366c 00000040
> 8056ce50
> GPR16: 805bc2c0 00010000 00000000 8056a9f0 00000010 00029000 80591f74
> 8056ce50
> GPR24: 805bc2c0 ae8edf50 b1023c08 ae91b300 00000000 ae91b300 ae91b320
> 00000000
> NIP [b9118ee4] rtdev_remove_pack+0xbc/0x144 [rtnet]
> LR [b9188a4c] rt_packet_close+0xb4/0x24c [rtpacket]
> Call Trace:
> [ae8ede00] [800ed3fc] generic_file_aio_write+0x74/0xf0 (unreliable)
> [ae8ede10] [b9188a4c] rt_packet_close+0xb4/0x24c [rtpacket]
> [ae8ede50] [800e41b4] __rt_dev_close+0x2c8/0x6d8
> [ae8ede80] [800e8274] sys_rtdm_close+0x18/0x28
> [ae8ede90] [800a08d4] losyscall_event+0x110/0x35c
> [ae8eded0] [8007b494] __ipipe_dispatch_event+0x128/0x2d0
> [ae8edf30] [8000b204] __ipipe_syscall_root+0x8c/0x13c
> [ae8edf40] [80012ad0] DoSyscall+0x20/0x5c
> Instruction dump:
> 409efff4 7c421378 4bffffcc a803000a 3be0fff5 2f800000 41bd0030 81630004
> 3c000010 81230000 60000100 3be00000 <912b0000> 90030000 3c000020 60000200
> 
> 
> 
> I believe the problem arises when opening a socket with protocol type
> set to 0, and then binding the socket with a non-zero packet type.
> Because sock->protocol remains 0 the new protocol type is not
> registered. However in rt_packet_close(), pt->type is checked for
> "non-zeroness" and then deregistered, leading to a list_del on a node
> that isn't inserted.

Good catch! That is indeed broken.

> I propose following patch:
> 
> Index: af_packet.c
> ===================================================================
> RCS file:
> /usr/cvs/sw/3rd_party/XM-Linux/rtnet-0.9.12/stack/packet/af_packet.c,v
> retrieving revision 1.1
> diff -r1.1 af_packet.c
> 108c108
> <     pt->type = new_type;
> ---
>>     pt->type = sock->protocol = new_type;
> 
> 
> My sincere apologies for using cvs.
> 

Well, I bet that wasn't your "wise" decision. :) But using unified diff
format (-u) would have been nice nevertheless.

Anyway, this is a more appropriate fix:

diff --git a/stack/packet/af_packet.c b/stack/packet/af_packet.c
index 5ca69e5..937e917 100644
--- a/stack/packet/af_packet.c
+++ b/stack/packet/af_packet.c
@@ -109,7 +109,7 @@ int rt_packet_bind(struct rtsocket *sock, const
struct sockaddr *addr,
     sock->prot.packet.ifindex = sll->sll_ifindex;

     /* if protocol is non-zero, register the packet type */
-    if (sock->protocol != 0) {
+    if (new_type != 0) {
         pt->handler     = rt_packet_rcv;
         pt->err_handler = NULL;


sock->protocol should remain untouched, it's the default protocol of
this socket. This fix is now also available through git.

Thanks for reporting!
Jan