[PATCH 1/1] net/x25: fix address parsing bug in x25_parse_address_block.

[PATCH 1/1] net/x25: fix address parsing bug in x25_parse_address_block.

[Stephen Moorby]

This problem was discovered when a linux box was incorrectly rejecting
calls from some X.25 equipment.  The problem was diagnosed to an incorrect
address length calculation in 'x25_parse_address_block', the calculation
did not account for the address digits being BCD encoded.  The correct
calculation is already performed on line 155.

Patched on linux-next 18-Jun-2013
Tested on 2.6.32-45-generic

Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>
---
 net/x25/af_x25.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 1d964e2..eb6c1f9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -98,7 +98,8 @@ int x25_parse_address_block(struct sk_buff *skb,
 	}
 
 	len = *skb->data;
-	needed = 1 + (len >> 4) + (len & 0x0f);
+	/* need 1 for address length + bytes for BCD encoding of 2 addresses */
+	needed = 1 + (((len >> 4) + (len & 0x0f) + 1) >> 1);
 
 	if (!pskb_may_pull(skb, needed)) {
 		/* packet is too short to hold the addresses it claims
-- 
1.8.1.2

Re: [PATCH 1/1] net/x25: fix address parsing bug in x25_parse_address_block.

[David Miller]

From: Stephen Moorby <steve.moorby@ntlworld.com>
Date: Wed, 19 Jun 2013 21:32:36 +0100

> This problem was discovered when a linux box was incorrectly rejecting
> calls from some X.25 equipment.  The problem was diagnosed to an incorrect
> address length calculation in 'x25_parse_address_block', the calculation
> did not account for the address digits being BCD encoded.  The correct
> calculation is already performed on line 155.
> 
> Patched on linux-next 18-Jun-2013
> Tested on 2.6.32-45-generic
> 
> Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>

This change has two problems:

1) If there is existing code that does the calculation correctly, don't
   get creative and express the calculation differently than the existing
   code.

2) If there are two places doing the same thing, write a helper function
   that does it in one place.

The exact reason this bug exists is because of code duplication, you are
making it even worse by writing the same calculation two different ways.

Please fix this up and resubmit, thanks.

[PATCH 1/1] net/x25: fix address parsing bug in x25_parse_address_block.

[Stephen Moorby]

 From 01ba835311f6d2e2bbffc74703d704dc07803b53 Mon Sep 17 00:00:00 2001
From: Stephen Moorby <steve.moorby@ntlworld.com>
Date: Wed, 19 Jun 2013 11:48:53 +0100
Subject: [PATCH 1/1] net/x25: fix address parsing bug in
  x25_parse_address_block.

This problem was discovered when a linux box was incorrectly rejecting
calls from some X.25 equipment.  The problem was diagnosed to an incorrect
address length calculation in 'x25_parse_address_block', the calculation
did not account for the address digits being BCD encoded.  The correct
calculation is already performed on line 155.

Patched on linux-next 18-Jun-2013
Tested on 2.6.32-45-generic

Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>
---
  net/x25/af_x25.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 1d964e2..eb6c1f9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -98,7 +98,8 @@ int x25_parse_address_block(struct sk_buff *skb,
      }

      len = *skb->data;
-    needed = 1 + (len >> 4) + (len & 0x0f);
+    /* need 1 for address length + bytes for BCD encoding of 2 addresses */
+    needed = 1 + (((len >> 4) + (len & 0x0f) + 1) >> 1);

      if (!pskb_may_pull(skb, needed)) {
          /* packet is too short to hold the addresses it claims
-- 
1.8.1.2