[PATCH] X.25: Fix address field length calculation

[PATCH] X.25: Fix address field length calculation

[Kelleter, Günther]

Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.

Signed-off-by: Guenther Kelleter <gkelleter@datus.com>
---
Wrong length calculation leads to rejection of CALL ACCEPT packets.

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

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 45a3ab5..2daf224 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -98,7 +98,7 @@ int x25_parse_address_block(struct sk_buff *skb,
 	}
  	len = *skb->data;
-	needed = 1 + (len >> 4) + (len & 0x0f);
+	needed = 1 + ((len >> 4) + (len & 0x0f) + 1) / 2;
  	if (!pskb_may_pull(skb, needed)) {
 		/* packet is too short to hold the addresses it claims
-- 
1.8.3.4.71.g0878476

Re: [PATCH] X.25: Fix address field length calculation

[Joe Perches]

On Tue, 2013-10-15 at 14:29 +0000, Kelleter, Günther wrote:
> Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
[]
> Wrong length calculation leads to rejection of CALL ACCEPT packets.
[]
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
[]
> @@ -98,7 +98,7 @@ int x25_parse_address_block(struct sk_buff *skb,
>  	}
>   	len = *skb->data;
> -	needed = 1 + (len >> 4) + (len & 0x0f);
> +	needed = 1 + ((len >> 4) + (len & 0x0f) + 1) / 2;

This calculation looks odd.
Perhaps use bcd.h instead?

Re: [PATCH] X.25: Fix address field length calculation

[Kelleter, Günther]

Am 15.10.2013 19:29, schrieb Joe Perches:
> On Tue, 2013-10-15 at 14:29 +0000, Kelleter, Günther wrote:
>> Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
> []
>> Wrong length calculation leads to rejection of CALL ACCEPT packets.
> []
>> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> []
>> @@ -98,7 +98,7 @@ int x25_parse_address_block(struct sk_buff *skb,
>>  	}
>>   	len = *skb->data;
>> -	needed = 1 + (len >> 4) + (len & 0x0f);
>> +	needed = 1 + ((len >> 4) + (len & 0x0f) + 1) / 2;
> This calculation looks odd.
> Perhaps use bcd.h instead?
>

It's just the same calculation as in x25_add_ntoa (last line) and it's
used this way by x.25.
Two digits are encoded to one byte and the last byte is padded with 0 if
the total number of digits is odd.--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] X.25: Fix address field length calculation

[David Laight]

> On Tue, 2013-10-15 at 14:29 +0000, Kelleter, Günther wrote:
> > Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
> []
> > Wrong length calculation leads to rejection of CALL ACCEPT packets.
> []
> > diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> []
> > @@ -98,7 +98,7 @@ int x25_parse_address_block(struct sk_buff *skb,
> >  	}
> >   	len = *skb->data;
> > -	needed = 1 + (len >> 4) + (len & 0x0f);
> > +	needed = 1 + ((len >> 4) + (len & 0x0f) + 1) / 2;
> 
> This calculation looks odd.

Looks correct to me...
In X.25 the lengths (in digits) of the called and calling addresses
are encoded in the high and low nibbles of one byte and then
followed by both addresses with a digit in each nibble.
If the length of the first address is odd, the second one
isn't byte aligned.

	David

Re: [PATCH] X.25: Fix address field length calculation

[Andrew Hendry]

Sorry for the previous html mail.
This appears to be correct, what length addresses are you getting back
in the call accept when this happens?

On Wed, Oct 16, 2013 at 7:56 PM, David Laight <David.Laight@aculab.com> wrote:
>> On Tue, 2013-10-15 at 14:29 +0000, Kelleter, Günther wrote:
>> > Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
>> []
>> > Wrong length calculation leads to rejection of CALL ACCEPT packets.
>> []
>> > diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
>> []
>> > @@ -98,7 +98,7 @@ int x25_parse_address_block(struct sk_buff *skb,
>> >     }
>> >     len = *skb->data;
>> > -   needed = 1 + (len >> 4) + (len & 0x0f);
>> > +   needed = 1 + ((len >> 4) + (len & 0x0f) + 1) / 2;
>>
>> This calculation looks odd.
>
> Looks correct to me...
> In X.25 the lengths (in digits) of the called and calling addresses
> are encoded in the high and low nibbles of one byte and then
> followed by both addresses with a digit in each nibble.
> If the length of the first address is odd, the second one
> isn't byte aligned.
>
>         David
>
>
>

Re: [PATCH] X.25: Fix address field length calculation

[Kelleter, Günther]

E.g. called address 7 digits and caller address 3 digits. Called DCE
answering without facilities
gives us this packet (hex):

37 12 34 56 71 23 00

then x25_parse_address_block()  tries to pull 1+7+3 = 11 bytes from the
packet (with pskb_may_pull())
which only has 7 bytes.
When facilities are included the wrong calculated length has no effect
since the facilities make this packet long enough to make pskb_may_pull
with wrong number
of bytes succeed. later x25_addr_ntoa() correctly pulls 6 bytes for
addresses from the packet.


Am 17.10.2013 13:02, schrieb Andrew Hendry:
> Sorry for the previous html mail.
> This appears to be correct, what length addresses are you getting back
> in the call accept when this happens?
>
> On Wed, Oct 16, 2013 at 7:56 PM, David Laight <David.Laight@aculab.com> wrote:
>>> On Tue, 2013-10-15 at 14:29 +0000, Kelleter, Günther wrote:
>>>> Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
>>> []
>>>> Wrong length calculation leads to rejection of CALL ACCEPT packets.
>>> []
>>>> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
>>> []
>>>> @@ -98,7 +98,7 @@ int x25_parse_address_block(struct sk_buff *skb,
>>>>     }
>>>>     len = *skb->data;
>>>> -   needed = 1 + (len >> 4) + (len & 0x0f);
>>>> +   needed = 1 + ((len >> 4) + (len & 0x0f) + 1) / 2;
>>> This calculation looks odd.
>> Looks correct to me...
>> In X.25 the lengths (in digits) of the called and calling addresses
>> are encoded in the high and low nibbles of one byte and then
>> followed by both addresses with a digit in each nibble.
>> If the length of the first address is odd, the second one
>> isn't byte aligned.
>>
>>         David
>>
>>
>>


-- 

Re: [PATCH] X.25: Fix address field length calculation

[David Miller]

From: Kelleter, Günther <GKelleter@datus.com>
Date: Tue, 15 Oct 2013 14:29:06 +0000

> Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
> 
> Signed-off-by: Guenther Kelleter <gkelleter@datus.com>
> ---
> Wrong length calculation leads to rejection of CALL ACCEPT packets.

This patch doesn't apply because it was severely corrupted by your
email client, turn off all encodings etc. in your client, send
a test patch to yourself, and do not submit this patch again until
you can successfully apply the patch you receive in those test emails.

Thanks.