ppp_mppe: account for osize too small errors in mppe_decompress()
Subsystems (2)
PATCH 1/1 ppp_mppe: account for osize too small errors in mppe_decompress()
Prevent mppe_decompress() from generating "osize too small" errors when checking for output buffer size. When receiving a packet of mru size the output buffer for decrypted data is 1 byte too small since mppe_decompress() tries to account for possible PFC, however later in code it is assumed no PFC.
Adjusting the check prevented there errors from occurring.
---
drivers/net/ppp_mppe.c renamed from drivers/net/ppp_mppe.c.orig +5 -5
@@ -493,14 +493,14 @@ mppe_decompress(void *arg, unsigned char/* * Make sure we have enough room to decrypt the packet. -* Note that for our test we only subtract 1 byte whereas in -* mppe_compress() we added 2 bytes (+MPPE_OVHD); -* this is to account for possible PFC. +* To account for possible PFC we should only subtract 1 +* byte whereas in mppe_compress() we added 2 bytes (+MPPE_OVHD); +* However, we assume no PFC, thus subtracting 2 bytes. */ -if (osize < isize -MPPE_OVHD -1 ) { +if (osize < isize -MPPE_OVHD -2 ) {printk (KERN_DEBUG "mppe_decompress[%d]: osize too small! " "(have: %d need: %d)\n" , state->unit, - osize, isize -MPPE_OVHD -1 ); + osize, isize -MPPE_OVHD -2 );return DECOMP_ERROR ; } osize = isize -MPPE_OVHD -2 ;/* assume no PFC */
Quoted 21 lines · On Tue, 05 Jun 2007 22:23:02 +1100 Konstantin Sharlaimov wrote:
Prevent
mppe_decompress()from generating "osize too small" errors when checking for output buffer size. When receiving a packet of mru size the output buffer for decrypted data is 1 byte too small sincemppe_decompress()tries to account for possible PFC, however later in code it is assumed no PFC.Adjusting the check prevented there errors from occurring.
Signed-off-by Konstantin Sharlaimov <[email protected]>---
drivers/net/ppp_mppe.c renamed from drivers/net/ppp_mppe.c.orig +3
-3@@ -493,14 +493,14 @@ mppe_decompress(void *arg, unsigned char/* * Make sure we have enough room to decrypt the packet. -* Note that for our test we only subtract 1 byte whereas in -* mppe_compress() we added 2 bytes (+MPPE_OVHD); -* this is to account for possible PFC. +* To account for possible PFC we should only subtract 1 +* byte whereas in mppe_compress() we added 2 bytes (+MPPE_OVHD); +* However, we assume no PFC, thus subtracting 2 bytes.
drivers/net/ppp_mppe.c mppe_decompress(void *arg, unsigned char · line 506
Quoted 10 lines
drivers/net/ppp_mppe.c renamed from drivers/net/ppp_mppe.c.orig +2
-2*/ -if (osize < isize -MPPE_OVHD -1 ) { +if (osize < isize -MPPE_OVHD -2 ) {printk (KERN_DEBUG "mppe_decompress[%d]: osize too small! " "(have: %d need: %d)\n" , state->unit, - osize, isize -MPPE_OVHD -1 ); + osize, isize -MPPE_OVHD -2 );return DECOMP_ERROR ; } osize = isize -MPPE_OVHD -2 ;/* assume no PFC */
Seems that this patch is not correct - even though the comment above says "assume no PFC", the subsequent code actually supports PFC:
/*
* Do PFC decompression.
* This would be nicer if we were given the actual sk_buff
* instead of a char *.
*/
if ((obuf[0] & 0x01) != 0) {
obuf[1] = obuf[0];
obuf[0] = 0;
obuf++;
osize++;
}Therefore, depending on the packet contents, the decompressor may really need that extra byte in the output buffer, and changing the check may cause buffer overflows.
Thanks for pointing this out. It seems that we would need to allocate this extra byte somewhere in kernel ppp code.
However, I am not very familiar with that part of the code yet, and I'll appreciate any help with finding parts of code where obuffer for mppe_decompress is allocated.
Quoted 54 lines · Sergey Vlasov
Quoted 21 lines · On Tue, 05 Jun 2007 22:23:02 +1100 Konstantin Sharlaimov wrote:
Prevent
mppe_decompress()from generating "osize too small" errors when checking for output buffer size. When receiving a packet of mru size the output buffer for decrypted data is 1 byte too small sincemppe_decompress()tries to account for possible PFC, however later in code it is assumed no PFC.Adjusting the check prevented there errors from occurring.
Signed-off-by Konstantin Sharlaimov <[email protected]>---
drivers/net/ppp_mppe.c 2007-06-05 21:46:30.000000000 +1100 renamed from drivers/net/ppp_mppe.c.orig 2007-06-01 20:57:04.000000000 +1100 +3
-3@@ -493,14 +493,14 @@ mppe_decompress(void *arg, unsigned char /* * Make sure we have enough room to decrypt the packet. - * Note that for our test we only subtract 1 byte whereas in - * mppe_compress() we added 2 bytes (+MPPE_OVHD); - * this is to account for possible PFC. + * To account for possible PFC we should only subtract 1 + * byte whereas in mppe_compress() we added 2 bytes (+MPPE_OVHD); + * However, we assume no PFC, thus subtracting 2 bytes.drivers/net/ppp_mppe.c 2007-06-05 21:46:30.000000000 +1100 mppe_decompress(void *arg, unsigned char · line 506
Quoted 10 lines
drivers/net/ppp_mppe.c 2007-06-05 21:46:30.000000000 +1100 renamed from drivers/net/ppp_mppe.c.orig 2007-06-01 20:57:04.000000000 +1100 +2
-2*/ - if (osize < isize - MPPE_OVHD - 1) { + if (osize < isize - MPPE_OVHD - 2) { printk(KERN_DEBUG "mppe_decompress[%d]: osize too small! " "(have: %d need: %d)\n", state->unit, - osize, isize - MPPE_OVHD - 1); + osize, isize - MPPE_OVHD - 2); return DECOMP_ERROR; } osize = isize - MPPE_OVHD - 2; /* assume no PFC */Seems that this patch is not correct - even though the comment above says "assume no PFC", the subsequent code actually supports PFC:
/* * Do PFC decompression. * This would be nicer if we were given the actual sk_buff * instead of a char *. */ if ((obuf[0] & 0x01) != 0) { obuf[1] = obuf[0]; obuf[0] = 0; obuf++; osize++; }Therefore, depending on the packet contents, the decompressor may really need that extra byte in the output buffer, and changing the check may cause buffer overflows.