Hi David,
please consider pulling from
git://eden-feed.erg.abdn.ac.uk/net-2.6
This set has bug-fixes only, non-bug fixes will follow at a later stage.
Some test-tree patches need to be reworked with regard to prefix conventions,
will send a request-for-comments later this week to enquire about this.
Patch #1: Fixes a divide-by-zero bug in CCID-3.
Patch #2: Resolves sparse warnings (gathered from several patches in the tree).
Patch #3: Enforces that Ack Vectors are not interpreted on request sockets.
Patch #4: Computation error in CCID-3 allowed sending rate.
Patch #5: Sending rate in CCID-3 truncated due to u64 -> u32 conversion.
Patch #6: Typo in initial sequence number assignment.
Patch #7: Bug in the initialisation of v4/v6 request-socket option areas.
Step 8.5 in RFC 4340 says for the newly cloned socket
Initialize S.GAR := S.ISS,
but what in fact the code (minisocks.c) does is
Initialize S.GAR := S.ISR,
which is wrong (typo?) -- fixed by the patch.
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/minisocks.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
From: Wei Yongjun <redacted>
From: Wei Yongjun <redacted>
This fixes the following bug:
* dccp_v4_reqsk_destructor() frees inet inet_rsk(req)->opt,
* but dccp_v4_conn_request() may fail before initialising inet_rsk(req)->opt;
* likewise, dccp_v6_reqsk_destructor() frees inet6_rsk(req)->pktopts,
* but dccp_v6_conn_request() may fail before initialising the pktopts.
The fix is in initialising the option areas in both request sockets before
calling any other code that may fail and thus may end up calling the destructor.
Signed-off-by: Wei Yongjun <redacted>
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/ipv4.c | 5 +++--
net/dccp/ipv6.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
Commit 825de27d9e40b3117b29a79d412b7a4b78c5d815 fixed the CCID-3 window counter
computation for RTTs < 4 microseconds (as happens on loopback).
Since it uses the term "1/RTT", it needs to be protected against divide-by-zero,
done in established state using dccp_sample_rtt().
But there was an oversight, as a zero RTT can happen on sender initialisation
when there is no RTT sample from the Request/Response exchange.
The fix is to use the fallback-RTT from RFC 4340, 3.4.
This is also better than just fixing update_win_count() since it allows other
parts of the code to always assume that a (fallback) RTT value is available.
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/ccids/ccid3.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
This fixes a bug in computing the inter-packet-interval t_ipi = s/X:
scaled_div32(a, b) uses u32 for b, but in "scaled_div32(s, X)" the type of the
sending rate `X' is u64. Since X is scaled by 2^6, this truncates rates greater
than 2^26 Bps (~537 Mbps).
Using full 64-bit division now.
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/ccids/lib/tfrc.h | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
@@ -15,7 +15,7 @@*(atyouroption)anylaterversion.*/#include<linux/types.h>-#include<asm/div64.h>+#include<linux/math64.h>#include"../../dccp.h"/* internal includes that this module exports: */#include"loss_interval.h"
@@ -29,21 +29,19 @@ extern int tfrc_debug;#endif/* integer-arithmetic divisions of type (a * 1000000)/b */-staticinlineu64scaled_div(u64a,u32b)+staticinlineu64scaled_div(u64a,u64b){BUG_ON(b==0);-a*=1000000;-do_div(a,b);-returna;+returndiv64_u64(a*1000000,b);}-staticinlineu32scaled_div32(u64a,u32b)+staticinlineu32scaled_div32(u64a,u64b){u64result=scaled_div(a,b);if(result>UINT_MAX){-DCCP_CRIT("Overflow: a(%llu)/b(%u) > ~0U",-(unsignedlonglong)a,b);+DCCP_CRIT("Overflow: %llu/%llu > UINT_MAX",+(unsignedlonglong)a,(unsignedlonglong)b);returnUINT_MAX;}returnresult;
This fixes an oversight from an earlier patch, ensuring that Ack Vectors
are not processed on request sockets.
The issue is that Ack Vectors must not be parsed on request sockets, since
the Ack Vector feature depends on the selection of the (TX) CCID. During the
initial handshake the CCIDs are undefined, and so RFC 4340, 10.3 applies:
"Using CCID-specific options and feature options during a negotiation
for the corresponding CCID feature is NOT RECOMMENDED [...]"
Worse, it is not even possible: when the server receives the Request from the
client, the CCID and Ack vector features are undefined; when the Ack finalising
the 3-way hanshake arrives, the request socket has not been cloned yet into a
full socket. (This order is necessary, since otherwise the newly created socket
would have to be destroyed whenever an option error occurred - a malicious
hacker could simply send garbage options and exploit this.)
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/options.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
This fixes a bug in the reverse lookup of p: given a value f(p), instead of p,
the function returned the smallest tabulated value f(p).
The smallest tabulated value of
10^6 * f(p) = sqrt(2*p/3) + 12 * sqrt(3*p/8) * (32 * p^3 + p)
for p=0.0001 is 8172.
Since this value is scaled by 10^6, the outcome of this bug is that a loss
of 8172/10^6 = 0.8172% was reported whenever the input was below the table
resolution of 0.01%.
This means that the value was over 80 times too high, resulting in large spikes
of the initial loss interval, thus unnecessarily reducing the throughput.
Also corrected the printk format (%u for u32).
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/ccids/lib/tfrc_equation.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
This patch fixes the following sparse warnings:
* nested min(max()) expression:
net/dccp/ccids/ccid3.c:91:21: warning: symbol '__x' shadows an earlier one
net/dccp/ccids/ccid3.c:91:21: warning: symbol '__y' shadows an earlier one
* Declaration of function prototypes in .c instead of .h file, resulting in
"should it be static?" warnings.
* Declared "struct dccpw" static (local to dccp_probe).
* Disabled dccp_delayed_ack() - not fully removed due to RFC 4340, 11.3
("Receivers SHOULD implement delayed acknowledgement timers ...").
* Used a different local variable name to avoid
net/dccp/ackvec.c:293:13: warning: symbol 'state' shadows an earlier one
net/dccp/ackvec.c:238:33: originally declared here
* Removed unused functions `dccp_ackvector_print' and `dccp_ackvec_print'.
Signed-off-by: Gerrit Renker <redacted>
---
net/dccp/ackvec.c | 29 ++---------------------------
net/dccp/ccids/ccid3.c | 4 ++--
net/dccp/ccids/lib/tfrc.c | 8 --------
net/dccp/ccids/lib/tfrc.h | 11 +++++++++--
net/dccp/output.c | 2 ++
net/dccp/probe.c | 2 +-
6 files changed, 16 insertions(+), 40 deletions(-)
@@ -508,6 +508,7 @@ void dccp_send_ack(struct sock *sk)EXPORT_SYMBOL_GPL(dccp_send_ack);+#if 0/* FIXME: Is this still necessary (11.3) - currently nowhere used by DCCP. */voiddccp_send_delayed_ack(structsock*sk){
From: Arnaldo Carvalho de Melo <hidden> Date: 2008-06-10 14:38:27
Em Tue, Jun 10, 2008 at 01:53:43PM +0100, Gerrit Renker escreveu:
From: Wei Yongjun <redacted>
From: Wei Yongjun <redacted>
This fixes the following bug:
* dccp_v4_reqsk_destructor() frees inet inet_rsk(req)->opt,
* but dccp_v4_conn_request() may fail before initialising inet_rsk(req)->opt;
* likewise, dccp_v6_reqsk_destructor() frees inet6_rsk(req)->pktopts,
* but dccp_v6_conn_request() may fail before initialising the pktopts.
The fix is in initialising the option areas in both request sockets before
calling any other code that may fail and thus may end up calling the destructor.
Signed-off-by: Wei Yongjun <redacted>
Signed-off-by: Gerrit Renker <redacted>
The problem is present in TCP too, look at cookie_v4_check and
tcp_v4_conn_request, we may get to reqsk_free before we set ->opt if
security_inet_conn_request() returns non zero.
This is one case where bugs found on DCCP lead to an audit of the
similar code in TCP and the fix should be done on the common
infrastructure.
Please consider the following patch instead, it is compile tested only
but should be OK.
Thanks a lot,
- Arnaldo
commit f627bdaa7428f04b828abd70a8145cafb7ce366b
Author: Arnaldo Carvalho de Melo [off-list ref]
Date: Tue Jun 10 10:37:32 2008 -0300
inet{6}_request_sock: Init ->opt and ->pktopts in the constructor
Wei Yongjun noticed that we may call reqsk_free on request sock objects where
the opt fields may not be initialized, fix it by introducing inet_reqsk_alloc
where we initialize ->opt to NULL and set ->pktopts to NULL in
inet6_reqsk_alloc.
Cc: Gerrit Renker [off-list ref]
Cc: Wei Yongjun [off-list ref]
Signed-off-by: Arnaldo Carvalho de Melo [off-list ref]
please consider pulling from
git://eden-feed.erg.abdn.ac.uk/net-2.6
This set has bug-fixes only, non-bug fixes will follow at a later stage.
When specifying a URL for me to pull from, you have to
specify which branch I should pull from even if it is
plainly just "master".
GIT requires this specification when pulling, and you
can eliminate all doubt by letting me know which branch
to pull from explicitly.
In any event, I'm not pulling this because I saw some
things that I want you to cleanup first.
Commit 825de27d9e40b3117b29a79d412b7a4b78c5d815 fixed the CCID-3 window counter
computation for RTTs < 4 microseconds (as happens on loopback).
When referencing GIT SHA1 IDs in commit logs, always
provide the commit changelog header line text like
this:
In commit $(SHA1_ID) ("[DCCP]: blah blah blah") we
did whatever...
because if this patch is ported into another GIT tree,
the SHA1_ID of the referenced commit might be different
and the commit header line text helps people find the
correct change you are referring to even if this happens.
I say this to someone at least one time every day that I
integrate patches. Please be mindful of this so I don't
have to ask you to do it again, thanks.
Hi David,
please consider the revised set of DCCP Bug-Fix patches
(changelog below), which can be pulled from:
git://eden-feed.erg.abdn.ac.uk/net-2.6
All patches are in branch `master', on top of a freshly cloned net-2.6 tree.
Result has been compile-tested again.
List of changes introduced in this submission
---------------------------------------------
Relative to yesterday's submission, only these things have changed:
* rewrote and updated the commit message of the first patch, which was
referencing a SHA1 ID only. This now has data and commit information added;
* removed the request-sock initialisation patch, since this has already been
fixed in your tree.
View-ability of changes
-----------------------
Since there are no code changes, I omit sending all the patches again. But
the differences can in any case be viewed on
http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=net-2.6.git;a=summary
List of patches included in this set:
-------------------------------------
Patch #1: Fixes a divide-by-zero bug in CCID-3.
Patch #2: Resolves sparse warnings (gathered from several patches in the tree).
Patch #3: Enforces that Ack Vectors are not interpreted on request sockets.
Patch #4: Computation error in CCID-3 allowed sending rate.
Patch #5: Sending rate in CCID-3 truncated due to u64 -> u32 conversion.
Patch #6: Typo in initial sequence number assignment.
please consider the revised set of DCCP Bug-Fix patches
(changelog below), which can be pulled from:
git://eden-feed.erg.abdn.ac.uk/net-2.6
All patches are in branch `master', on top of a freshly cloned net-2.6 tree.
Result has been compile-tested again.