[PATCH] issues with printk "mppe_compress[%d]: osize too small! ..."

[PATCH] issues with printk "mppe_compress[%d]: osize too small! ..."

[Helmut Grohne]

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

Dear kernel maintainers,

I ran into an issue that causes "mppe_compress[%d]: osize too small! …"
message to be printed. While debugging the issue, I stumbled across two
additional issues with the code in question.

1) The printk gives a "have" and a "need" value. However the "need"
   value does not represent the actual need, because the printed need
   is based on the osize variable instead of the isize variable used in
   the test. This appears to by a typo an is as old as the ppp_mppe.c
   file. The first patch attached changes the parameter to printk to
   print the actual need.

2) The message in question can be triggered by network packets. If that
   happens, there are a lot of printks and the whole system slows down
   to a crawl. Whenever network packets are processed, the printks must
   be rate limited in some way, so my second patch adds a call to
   net_ratelimit(). Note that even though checkpatch suggested to
   convert the printks to netdev_dbg, I felt that a bug fix should be
   minimal and thus not change this aspect.

Please consider these patches and CC me in replies.

Helmut


[-- Attachment #2: 0001-mppe-print-correct-osize-required-for-compression.patch --]
[-- Type: application/octet-stream, Size: 1270 bytes --]

From 77ef588c202c1e8b23f5292e6b64d7cd69a9daba Mon Sep 17 00:00:00 2001
From: Helmut Grohne <h.grohne@cygnusnetworks.de>
Date: Fri, 11 Oct 2013 14:18:50 +0200
Subject: [PATCH] mppe: print correct osize required for compression

An example message looks like this:

mppe_compress[76]: osize too small! (have: 1404 need: 1408)

The value printed as "need" is not the one that is actually needed,
because it is based on osize instead of isize as used in the test. In
the old implementation the "need" value would always 4 greater than the
"have" value.

Signed-off-by: Helmut Grohne <h.grohne@cygnusnetworks.de>
---
 drivers/net/ppp/ppp_mppe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 9a1849a..3d595a8 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -385,7 +385,7 @@ mppe_compress(void *arg, unsigned char *ibuf, unsigned char *obuf,
 		/* Drop the packet if we should encrypt it, but can't. */
 		printk(KERN_DEBUG "mppe_compress[%d]: osize too small! "
 		       "(have: %d need: %d)\n", state->unit,
-		       osize, osize + MPPE_OVHD + 2);
+		       osize, isize + MPPE_OVHD + 2);
 		return -1;
 	}
 
-- 
1.7.10.4


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



[-- Attachment #4: 0001-mppe-rate-limit-osize-too-small.patch --]
[-- Type: application/octet-stream, Size: 2557 bytes --]

From 3332d572e21f52d65df841ba85537b65d336007e Mon Sep 17 00:00:00 2001
From: Helmut Grohne <h.grohne@cygnusnetworks.de>
Date: Fri, 11 Oct 2013 14:34:06 +0200
Subject: [PATCH] mppe: rate limit "osize too small"

These messages occur per packet and can be triggered by remote sites. As
such they must be rate limited in order to not clog the kernel log
buffers. The corresponding messages from ppp_generic.c are already rate
limited. Example:

mppe_compress[76]: osize too small! (have: 1404 need: 1408)
ppp: compressor dropped pkt
mppe_compress[76]: osize too small! (have: 1404 need: 1408)
ppp: compressor dropped pkt
mppe_compress[76]: osize too small! (have: 1404 need: 1408)
mppe_compress[76]: osize too small! (have: 1404 need: 1408)

Signed-off-by: Helmut Grohne <h.grohne@cygnusnetworks.de>
---
 drivers/net/ppp/ppp_mppe.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 9a1849a..7e71a7a 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -52,6 +52,7 @@
 #include <linux/string.h>
 #include <linux/crypto.h>
 #include <linux/mm.h>
+#include <linux/net.h>
 #include <linux/ppp_defs.h>
 #include <linux/ppp-comp.h>
 #include <linux/scatterlist.h>
@@ -383,9 +384,10 @@ mppe_compress(void *arg, unsigned char *ibuf, unsigned char *obuf,
 	/* Make sure we have enough room to generate an encrypted packet. */
 	if (osize < isize + MPPE_OVHD + 2) {
 		/* Drop the packet if we should encrypt it, but can't. */
-		printk(KERN_DEBUG "mppe_compress[%d]: osize too small! "
-		       "(have: %d need: %d)\n", state->unit,
-		       osize, osize + MPPE_OVHD + 2);
+		if (net_ratelimit())
+			printk(KERN_DEBUG "mppe_compress[%d]: osize too small! "
+			       "(have: %d need: %d)\n", state->unit,
+			       osize, osize + MPPE_OVHD + 2);
 		return -1;
 	}
 
@@ -497,9 +499,10 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	 * this is to account for possible PFC.
 	 */
 	if (osize < isize - MPPE_OVHD - 1) {
-		printk(KERN_DEBUG "mppe_decompress[%d]: osize too small! "
-		       "(have: %d need: %d)\n", state->unit,
-		       osize, isize - MPPE_OVHD - 1);
+		if (net_ratelimit())
+			printk(KERN_DEBUG "mppe_decompress[%d]: osize too small! "
+			       "(have: %d need: %d)\n", state->unit,
+			       osize, isize - MPPE_OVHD - 1);
 		return DECOMP_ERROR;
 	}
 	osize = isize - MPPE_OVHD - 2;	/* assume no PFC */
-- 
1.7.10.4