kern/105943: Network stack may modify read-only mbuf chain copies

[Yar Tikhiy]

>Number:         105943
>Category:       kern
>Synopsis:       Network stack may modify read-only mbuf chain copies
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Nov 28 10:20:09 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Yar Tikhiy
>Release:        FreeBSD 7.0-CURRENT i386
>Organization:
none
>Environment:
	
	At least, all versions found in the CVS repository.

>Description:
	
	The m_copym() function and its historic alias m_copy() are
	used all over the kernel to get a copy of a mbuf chain.
	However, at least some code fails to account for the fact
	that the copy is read-only due to the mbuf clusters from
	the original chain not actually copied, but merely referenced
	by the new chain.

	The most obvious case is the routines calling if_simloop()
	to return a copy of a network packet up the stack.  It is
	done to let the stack see its own broadcast traffic if the
	outgoing interface is simplex.  Such routines all use
	m_copym() to make the copy, but the upper layers of the
	stack are not prohibited from modifying the mbuf chain they
	get on input.  Consequently, the original chain ready to
	be transmitted to the network may be damaged unpredictably.
	The damage can have various consequences, from broken packets
	sent to system crashes.

	In addition, there might be more places in the kernel where
	m_copym() or m_copy() is used although a writable copy is
	needed.

>How-To-Repeat:
	
	Although the problem didn't manifested itself for ages, a
	recent change in the distribution of data over a mbuf chain
	has finally triggered it in CURRENT.  A stock generator of
	broadcast traffic is rwhod(8).  As soon as its packet is
	longer than ~256 bytes and so it doesn't fit in one simple
	mbuf, it appears damaged on the network.  To reach the
	threshold length, just open multiple terminal sessions to
	the system.

16:15:28.212810 IP truncated-ip - 6865 bytes missing! (tos 0x0, ttl  64, id 28554, offset 0, \
                flags [none], proto: UDP (17), length: 7169, bad cksum 11c (->c64b)!) \
                10.10.10.4.513 > 10.10.10.255.513: UDP, length 276
        0x0000:  4500 1c01 6f8a 0000 4011 011c 0a0a 0a04  E...o... at .......
                      ^^^^                ^^^^ broken fields
         0x0010:  0a0a 0aff 0201 0201 011c 0000 0101 0000  ................
         0x0020:  4565 9ef0 0000 0000 6467 0000 0000 0000  Ee......dg......
         0x0030:  0000 0000 0000 0000 0000 0000 0000 0000  ................
         0x0040:  0000 0000 0000 0000 0000 001a 0000 000e  ................
         0x0050:  0000 0005 4564 a5e7 7474 7976 3000 0000  ....Ed..ttyv0...
         0x0060:  726f 6f74 0000 0000 4565 9d4a 0000 01a6  root....Ee.J....
         0x0070:  7474 7976 3100 0000 726f 6f74 0000 0000  ttyv1...root....
         0x0080:  4565 9d4d 0000 000c 7474 7976 3200 0000  Ee.M....ttyv2...
         0x0090:  726f 6f74 0000 0000 4565 9d4f 0000 0099  root....Ee.O....
         0x00a0:  7474 7976 3300 0000 726f 6f74 0000 0000  ttyv3...root....
         0x00b0:  4565 9d52 0000 019e 7474 7976 3400 0000  Ee.R....ttyv4...
         0x00c0:  726f 6f74 0000 0000 4565 9d54 0000 019c  root....Ee.T....
         0x00d0:  7474 7976 3500 0000 726f 6f74 0000 0000  ttyv5...root....
         0x00e0:  4565 9d59 0000 0198 7474 7976 3600 0000  Ee.Y....ttyv6...
         0x00f0:  726f 6f74 0000 0000 4565 9d5b 0000 0195  root....Ee.[....
         0x0100:  7474 7976 3700 0000 726f 6f74 0000 0000  ttyv7...root....
         0x0110:  4565 9d5e 0000 0000 7474 7970 3100 0000  Ee.^....ttyp1...
         0x0120:  7961 7200 0000 0000 4565 8361 0000 04b2  yar.....Ee.a....

>Fix:

	The initial idea is to drop all the bogus and often buggy
	code around if_simloop() calls and add a flag to if_simloop()
	indicating that the mbuf chain must be fully duplicated
	with m_dup() and then the copy is to be processed.  In
	STABLE, the flag can be indicated by adding M_COPYALL to
	the hlen argument.  M_COPYALL is a very large value that
	allows for the trick.  So ABI will be preserved in STABLE.

	At the same time, the other cases of m_copym() usage should
	be revised thoroughly.
>Release-Note:
>Audit-Trail:
>Unformatted: