2.6.25-stable review

This is the start of the stable review cycle for the 2.6.25.13 release. There are 9 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let us know. If anyone is a maintainer of the proper subsystem, and wants to add a Signed-off-by: line to the patch, please respond with it.

These patches are sent out with a number of different people on the Cc:
line. If you wish to be a reviewer, please email [email protected] to add your name to the list. If you want to be off the reviewer list, also email us.

Responses should be made by Monday, July 28, 12:00:00 UTC. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

	kernel.org/pub/linux/kernel/v2.6/stable-review/patch-2.6.25.13-rc1.gz

and the diffstat can be found below.

thanks,

the -stable release team

--------------

 Documentation/networking/udplite.txt |    2 -
 Makefile                             |    2 -
 drivers/net/hamradio/hdlcdrv.c       |    1 
 drivers/net/pppoe.c                  |   37 ++++++++++++++++++-----------------
 drivers/net/pppol2tp.c               |   20 +++++++++++-------
 include/linux/ipv6.h                 |    2 +
 include/linux/kernel.h               |    1 
 net/ipv4/raw.c                       |    2 -
 net/ipv4/udp.c                       |    4 +++
 net/ipv4/xfrm4_mode_tunnel.c         |    2 -
 net/ipv6/ip6_fib.c                   |    4 +--
 11 files changed, 46 insertions(+), 31 deletions(-)

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Micah Dowty <[email protected]>

[ Upstream commit ae6134bdf3197206fba95563d755d2fa50d90ddd ]

This is a trivial patch against the hdlcdrv module that fixes its CRC calculation. The finished CRC was overwriting the first two bytes of each packet rather than being appended to the end.

I've tested this with 2.6.8 and 2.6.10-rc1, but hdlcdrv hasn't changed much recently so it should work with many other kernel versions.

---
 drivers/net/hamradio/hdlcdrv.c |    1 +
 1 file changed, 1 insertion(+)

@@ -88,6 +88,7 @@
 static inline void append_crc_ccitt(unsigned char *buffer, int len)
 {
  	unsigned int crc = crc_ccitt(0xffff, buffer, len) ^ 0xffff;
+	buffer += len;
 	*buffer++ = crc;
 	*buffer++ = crc >> 8;
 }

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: David S. Miller <[email protected]>

[ Upstream commit ebb36a978131810c98e7198b1187090c697cf99f ]

Based upon a report by Olaf Hering.

---
 include/linux/ipv6.h |    2 ++
 1 file changed, 2 insertions(+)

@@ -123,6 +123,7 @@ struct ipv6hdr {
 	struct	in6_addr	daddr;
 };
 
+#ifdef __KERNEL__
 /*
  * This structure contains configuration options per IPv6 link.
  */

@@ -162,6 +163,7 @@ struct ipv6_devconf {
 #endif
 	void		*sysctl;
 };
+#endif
 
 /* index values for the variables in ipv6_devconf */
 enum {

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Stephen Hemminger <[email protected]>

[ Upstream commit 847499ce71bdcc8fc542062df6ebed3e596608dd ]

This fixes the bridge reference count problem and cleanups ipv6 FIB timer management. Don't use expires field, because it is not a proper way to test, instead use timer_pending().

---
 net/ipv6/ip6_fib.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -679,7 +679,7 @@ static int fib6_add_rt2node(struct fib6_
 
 static __inline__ void fib6_start_gc(struct rt6_info *rt)
 {
-	if (ip6_fib_timer.expires == 0 &&
+	if (!timer_pending(&ip6_fib_timer) &&
 	    (rt->rt6i_flags & (RTF_EXPIRES|RTF_CACHE)))
 		mod_timer(&ip6_fib_timer, jiffies +
 			  init_net.ipv6.sysctl.ip6_rt_gc_interval);

@@ -687,7 +687,7 @@ static __inline__ void fib6_start_gc(str
 
 void fib6_force_start_gc(void)
 {
-	if (ip6_fib_timer.expires == 0)
+	if (!timer_pending(&ip6_fib_timer))
 		mod_timer(&ip6_fib_timer, jiffies +
 			  init_net.ipv6.sysctl.ip6_rt_gc_interval);
 }

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: James Chapman <[email protected]>

[ Upstream commit 6b6707a50c7598a83820077393f8823ab791abf8 ]

This patch fixes a potential memory corruption in pppol2tp_recvmsg(). If skb->len is bigger than the caller's buffer length, memcpy_toiovec() will go into unintialized data on the kernel heap, interpret it as an iovec and start modifying memory.

The fix is to change the memcpy_toiovec() call to skb_copy_datagram_iovec() so that paged packets (rare for PPPOL2TP) are handled properly. Also check that the caller's buffer is big enough for the data and set the MSG_TRUNC flag if it is not so.

---
 drivers/net/pppol2tp.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

@@ -783,14 +783,18 @@ static int pppol2tp_recvmsg(struct kiocb
 	err = 0;
 	skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
 				flags & MSG_DONTWAIT, &err);
-	if (skb) {
-		err = memcpy_toiovec(msg->msg_iov, (unsigned char *) skb->data,
-				     skb->len);
-		if (err < 0)
-			goto do_skb_free;
-		err = skb->len;
-	}
-do_skb_free:
+	if (!skb)
+		goto end;
+
+	if (len > skb->len)
+		len = skb->len;
+	else if (len < skb->len)
+		msg->msg_flags |= MSG_TRUNC;
+
+	err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, len);
+	if (likely(err == 0))
+		err = len;
+
 	kfree_skb(skb);
 end:
 	return err;

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Herbert Xu <[email protected]>

[ Upstream commit 392fdb0e35055b96faa9c1cd6ab537805337cdce ]

The length field in the PPPOE header wasn't checked completely. This patch causes all packets shorter than the declared length to be dropped.

It also changes the memcpy_toiovec call to skb_copy_datagram_iovec so that paged packets (rare for PPPOE) are handled properly.

Thanks to Ilja of the Netric Security Team for discovering and reporting this bug, and Chris Wright for the total_len check.

[ Incorporate warning fix from Stephen Hemminger. -DaveM ]

---
 drivers/net/pppoe.c |   31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

@@ -341,12 +341,6 @@ static int pppoe_rcv_core(struct sock *s
 	struct pppox_sock *relay_po;
 
 	if (sk->sk_state & PPPOX_BOUND) {
-		struct pppoe_hdr *ph = pppoe_hdr(skb);
-		int len = ntohs(ph->length);
-		skb_pull_rcsum(skb, sizeof(struct pppoe_hdr));
-		if (pskb_trim_rcsum(skb, len))
-			goto abort_kfree;
-
 		ppp_input(&po->chan, skb);
 	} else if (sk->sk_state & PPPOX_RELAY) {
 		relay_po = get_item_by_addr(&po->pppoe_relay);

@@ -357,7 +351,6 @@ static int pppoe_rcv_core(struct sock *s
 		if ((sk_pppox(relay_po)->sk_state & PPPOX_CONNECTED) == 0)
 			goto abort_put;
 
-		skb_pull(skb, sizeof(struct pppoe_hdr));
 		if (!__pppoe_xmit(sk_pppox(relay_po), skb))
 			goto abort_put;
 	} else {

@@ -388,6 +381,7 @@ static int pppoe_rcv(struct sk_buff *skb
 {
 	struct pppoe_hdr *ph;
 	struct pppox_sock *po;
+	int len;
 
 	if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
 		goto out;

@@ -399,10 +393,21 @@ static int pppoe_rcv(struct sk_buff *skb
 		goto drop;
 
 	ph = pppoe_hdr(skb);
+	len = ntohs(ph->length);
+
+	skb_pull_rcsum(skb, sizeof(*ph));
+	if (skb->len < len)
+		goto drop;
 
 	po = get_item(ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
-	if (po != NULL)
-		return sk_receive_skb(sk_pppox(po), skb, 0);
+	if (!po)
+		goto drop;
+
+	if (pskb_trim_rcsum(skb, len))
+		goto drop;
+
+	return sk_receive_skb(sk_pppox(po), skb, 0);
+
 drop:
 	kfree_skb(skb);
 out:

@@ -937,12 +942,10 @@ static int pppoe_recvmsg(struct kiocb *i
 	m->msg_namelen = 0;
 
 	if (skb) {
-		struct pppoe_hdr *ph = pppoe_hdr(skb);
-		const int len = ntohs(ph->length);
-
-		error = memcpy_toiovec(m->msg_iov, (unsigned char *) &ph->tag[0], len);
+		total_len = min_t(size_t, total_len, skb->len);
+		error = skb_copy_datagram_iovec(skb, 0, m->msg_iov, total_len);
 		if (error == 0)
-			error = len;
+			error = total_len;
 	}
 
 	kfree_skb(skb);

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Herbert Xu <[email protected]>

[ Upstream commit bc6cffd177f9266af38dba96a2cea06c1e7ff932 ]

We need to unshare the skb first as otherwise pskb_may_pull may write to a shared skb which could be bad.

---
 drivers/net/pppoe.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

@@ -432,12 +432,12 @@ static int pppoe_disc_rcv(struct sk_buff
 	if (dev->nd_net != &init_net)
 		goto abort;
 
-	if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
-		goto abort;
-
 	if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
 		goto out;
 
+	if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
+		goto abort;
+
 	ph = pppoe_hdr(skb);
 	if (ph->code != PADT_CODE)
 		goto abort;

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Eric Dumazet <[email protected]>

[ Upstream commit 68be802cd5ad040fe8cfa33ce3031405df2d9117 ]

I just noticed "cat /proc/net/raw" was buggy, missing '\n' separators.

I believe this was introduced by commit 8cd850efa4948d57a2ed836911cfd1ab299e89c6 ([RAW]: Cleanup IPv4 raw_seq_show.)

This trivial patch restores correct behavior, and applies to current Linus tree (should also be applied to stable tree as well.)

---
 net/ipv4/raw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -936,7 +936,7 @@ static void raw_sock_seq_show(struct seq
 	      srcp  = inet->num;
 
 	seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
-		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d",
+		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n",
 		i, src, srcp, dest, destp, sp->sk_state,
 		atomic_read(&sp->sk_wmem_alloc),
 		atomic_read(&sp->sk_rmem_alloc),

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Steffen Klassert <[email protected]>

[ Upstream commit fe833fca2eac6b3d3ad5e35f44ad4638362f1da8 ]

When generating the ip header for the transformed packet we just copy the frag_off field of the ip header from the original packet to the ip header of the new generated packet. If we receive a packet as a chain of fragments, all but the last of the new generated packets have the IP_MF flag set. We have to mask the frag_off field to only keep the IP_DF flag from the original packet. This got lost with git commit 36cf9acf93e8561d9faec24849e57688a81eb9c5 ("[IPSEC]: Separate inner/outer mode processing on output")

---
 net/ipv4/xfrm4_mode_tunnel.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -52,7 +52,7 @@ static int xfrm4_mode_tunnel_output(stru
 		IP_ECN_clear(top_iph);
 
 	top_iph->frag_off = (flags & XFRM_STATE_NOPMTUDISC) ?
-			    0 : XFRM_MODE_SKB_CB(skb)->frag_off;
+		0 : (XFRM_MODE_SKB_CB(skb)->frag_off & htons(IP_DF));
 	ip_select_ident(top_iph, dst->child, NULL);
 
 	top_iph->ttl = dst_metric(dst->child, RTAX_HOPLIMIT);

2.6.25-stable review patch. If anyone has any objections, please let us know.

------------------
From: Gerrit Renker <[email protected]>

[ Upstream commit 47112e25da41d9059626033986dc3353e101f815 ]

This patch clamps the cscov setsockopt values to a maximum of 0xFFFF.

Setsockopt values greater than 0xffff can cause an unwanted wrap-around. Further, IPv6 jumbograms are not supported (RFC 3838, 3.5), so that values greater than 0xffff are not even useful.

Further changes: fixed a typo in the documentation.

[ Add USHORT_MAX from upstream to linux/kernel.h -DaveM ]

---
 Documentation/networking/udplite.txt |    2 +-
 include/linux/kernel.h               |    1 +
 net/ipv4/udp.c                       |    4 ++++
 3 files changed, 6 insertions(+), 1 deletion(-)

@@ -148,7 +148,7 @@
         getsockopt(sockfd, SOL_SOCKET, SO_NO_CHECK, &value, ...);
 
   is meaningless (as in TCP). Packets with a zero checksum field are
-  illegal (cf. RFC 3828, sec. 3.1) will be silently discarded.
+  illegal (cf. RFC 3828, sec. 3.1) and will be silently discarded.
 
   4) Fragmentation
 

@@ -20,6 +20,7 @@
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
+#define USHORT_MAX	((u16)(~0U))
 #define INT_MAX		((int)(~0U>>1))
 #define INT_MIN		(-INT_MAX - 1)
 #define UINT_MAX	(~0U)

@@ -1325,6 +1325,8 @@ int udp_lib_setsockopt(struct sock *sk, 
 			return -ENOPROTOOPT;
 		if (val != 0 && val < 8) /* Illegal coverage: use default (8) */
 			val = 8;
+		else if (val > USHORT_MAX)
+			val = USHORT_MAX;
 		up->pcslen = val;
 		up->pcflag |= UDPLITE_SEND_CC;
 		break;

@@ -1337,6 +1339,8 @@ int udp_lib_setsockopt(struct sock *sk, 
 			return -ENOPROTOOPT;
 		if (val != 0 && val < 8) /* Avoid silly minimal values.       */
 			val = 8;
+		else if (val > USHORT_MAX)
+			val = USHORT_MAX;
 		up->pcrlen = val;
 		up->pcflag |= UDPLITE_RECV_CC;
 		break;

Being very brave or very silly, I put this on my firewall box due to the PPPoE changes, I run ADSL modem in bridge mode, not noticed any problems :)

Cheers,
Grant.

Great, thanks for testing, I appreciate it.

greg k-h

While I appreciate keeping 2.6.25 going (I still need it on two boxen where 2.6.26 just doesn't play nicely), I'm kind of surprised to not see any similar activity against 2.6.26 - is .26 orphaned, or not started until after the merge window ?

Rick Nelson
Sorry.  I just realized this sentance makes no sense :)
		-- Ian Main

I'm just a bit slow in getting a .26 out. There are patches already queued up, and my mbox is quite full for more patches to go before it can get out. It was just really easy to flush the .25 patches out at this moment.

thanks,

greg "digging the inbox out after 3 weeks of travel" k-h

And thank you very much for doing that!

  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh