Commit ddd0451fc8dbf94446c81500ff0dcee06c4057cb
Committed by
David S. Miller
David S. Miller
1 parent
f5eb917b86
Exists in
master
and in
4 other branches
x.25 attempts to negotiate invalid throughput
The current X.25 code has some bugs in throughput negotiation:
1. It does negotiation in all cases, usually there is no need
2. It incorrectly attempts to negotiate the throughput class in one
direction only. There are separate throughput classes for input
and output and if either is negotiated both mist be negotiates.
This is bug https://bugzilla.kernel.org/show_bug.cgi?id=15681
This bug was first reported by Daniel Ferenci to the linux-x25 mailing
list on 6/8/2004, but is still present.
The current (2.6.34) x.25 code doesn't seem to know that the X.25
throughput facility includes two values, one for the required
throughput outbound, one for inbound.
This causes it to attempt to negotiate throughput 0x0A, which is
throughput 9600 inbound and the illegal value "0" for inbound
throughput.
Because of this some X.25 devices (e.g. Cisco 1600) refuse to connect
to Linux X.25.
The following patch fixes this behaviour. Unless the user specifies a
required throughput it does not attempt to negotiate. If the user
does not specify a throughput it accepts the suggestion of the remote
X.25 system. If the user requests a throughput then it validates both
the input and output throughputs and correctly negotiates them with
the remote end.
Signed-off-by: John Hughes <john@calva.com>
Tested-by: Andrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 2 changed files with 28 additions and 7 deletions Side-by-side Diff
net/x25/af_x25.c
| ... | ... | @@ -588,7 +588,8 @@ |
| 588 | 588 | x25->facilities.winsize_out = X25_DEFAULT_WINDOW_SIZE; |
| 589 | 589 | x25->facilities.pacsize_in = X25_DEFAULT_PACKET_SIZE; |
| 590 | 590 | x25->facilities.pacsize_out = X25_DEFAULT_PACKET_SIZE; |
| 591 | - x25->facilities.throughput = X25_DEFAULT_THROUGHPUT; | |
| 591 | + x25->facilities.throughput = 0; /* by default don't negotiate | |
| 592 | + throughput */ | |
| 592 | 593 | x25->facilities.reverse = X25_DEFAULT_REVERSE; |
| 593 | 594 | x25->dte_facilities.calling_len = 0; |
| 594 | 595 | x25->dte_facilities.called_len = 0; |
| ... | ... | @@ -1459,9 +1460,20 @@ |
| 1459 | 1460 | if (facilities.winsize_in < 1 || |
| 1460 | 1461 | facilities.winsize_in > 127) |
| 1461 | 1462 | break; |
| 1462 | - if (facilities.throughput < 0x03 || | |
| 1463 | - facilities.throughput > 0xDD) | |
| 1464 | - break; | |
| 1463 | + if (facilities.throughput) { | |
| 1464 | + int out = facilities.throughput & 0xf0; | |
| 1465 | + int in = facilities.throughput & 0x0f; | |
| 1466 | + if (!out) | |
| 1467 | + facilities.throughput |= | |
| 1468 | + X25_DEFAULT_THROUGHPUT << 4; | |
| 1469 | + else if (out < 0x30 || out > 0xD0) | |
| 1470 | + break; | |
| 1471 | + if (!in) | |
| 1472 | + facilities.throughput |= | |
| 1473 | + X25_DEFAULT_THROUGHPUT; | |
| 1474 | + else if (in < 0x03 || in > 0x0D) | |
| 1475 | + break; | |
| 1476 | + } | |
| 1465 | 1477 | if (facilities.reverse && |
| 1466 | 1478 | (facilities.reverse & 0x81) != 0x81) |
| 1467 | 1479 | break; |
net/x25/x25_facilities.c
| ... | ... | @@ -269,9 +269,18 @@ |
| 269 | 269 | new->reverse = theirs.reverse; |
| 270 | 270 | |
| 271 | 271 | if (theirs.throughput) { |
| 272 | - if (theirs.throughput < ours->throughput) { | |
| 273 | - SOCK_DEBUG(sk, "X.25: throughput negotiated down\n"); | |
| 274 | - new->throughput = theirs.throughput; | |
| 272 | + int theirs_in = theirs.throughput & 0x0f; | |
| 273 | + int theirs_out = theirs.throughput & 0xf0; | |
| 274 | + int ours_in = ours->throughput & 0x0f; | |
| 275 | + int ours_out = ours->throughput & 0xf0; | |
| 276 | + if (!ours_in || theirs_in < ours_in) { | |
| 277 | + SOCK_DEBUG(sk, "X.25: inbound throughput negotiated\n"); | |
| 278 | + new->throughput = (new->throughput & 0xf0) | theirs_in; | |
| 279 | + } | |
| 280 | + if (!ours_out || theirs_out < ours_out) { | |
| 281 | + SOCK_DEBUG(sk, | |
| 282 | + "X.25: outbound throughput negotiated\n"); | |
| 283 | + new->throughput = (new->throughput & 0x0f) | theirs_out; | |
| 275 | 284 | } |
| 276 | 285 | } |
| 277 | 286 |