2013-10-15 14:29:38

by Guenther Kelleter

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

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

Signed-off-by: Guenther Kelleter <[email protected]>
---
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


2013-10-15 17:29:55

by Joe Perches

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

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?

2013-10-16 06:59:00

by Guenther Kelleter

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

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.-

2013-10-16 08:58:53

by David Laight

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

> 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


2013-10-17 11:02:17

by Andrew Hendry

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

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 <[email protected]> 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
>
>
>

2013-10-17 12:09:49

by Guenther Kelleter

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


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 <[email protected]> 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
>>
>>
>>


--

2013-10-17 20:05:04

by David Miller

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

From: Kelleter, G?nther <[email protected]>
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 <[email protected]>
> ---
> 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.