[BUG-FIX][PATCH 0/2]: recent dccp fixes

[BUG-FIX][PATCH 0/2]: recent dccp fixes

[Gerrit Renker]

In January there were two dccp commits introducing bugs. Having
been guilty of ack-ing them, please find below patches to fix
them.

Patch #1: fixes a bug resulting from truncating cache name length.
Patch #2: reverts a commit which introduced a module-loading bug.

This being bugs, would you please consider this for stable too.

The test-tree also has been fixed accordingly,
http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=shortlog;h=ready    

[PATCH 01/86] dccp: fix bug in cache allocation

[Gerrit Renker]

This fixes a bug introduced in commit de4ef86cfce60d2250111f34f8a084e769f23b16
("dccp: fix dccp rmmod when kernel configured to use slub", 17 Jan): the
vsnprintf used sizeof(slab_name_fmt), which became truncated to 4 bytes, since
slab_name_fmt is now a 4-byte pointer and no longer a 32-character array.

This lead to error messages such as
 FATAL: Error inserting dccp: No buffer space available

 >> kernel: [ 1456.341501] kmem_cache_create: duplicate cache cci
generated due to the truncation after the 3rrdc character.

Fixed for the moment by introducing a symbolic constant. Tested to fix the bug.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/ccid.c |    2 +-
 net/dccp/ccid.h |    8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

--- a/net/dccp/ccid.c
+++ b/net/dccp/ccid.c
@@ -83,7 +83,7 @@ static struct kmem_cache *ccid_kmem_cache_create(int obj_size, char *slab_name_f
 	va_list args;
 
 	va_start(args, fmt);
-	vsnprintf(slab_name_fmt, sizeof(slab_name_fmt), fmt, args);
+	vsnprintf(slab_name_fmt, CCID_SLAB_NAME_LENGTH, fmt, args);
 	va_end(args);
 
 	slab = kmem_cache_create(slab_name_fmt, sizeof(struct ccid) + obj_size, 0,
--- a/net/dccp/ccid.h
+++ b/net/dccp/ccid.h
@@ -19,7 +19,9 @@
 #include <linux/list.h>
 #include <linux/module.h>
 
-#define CCID_MAX 255
+/* maximum value for a CCID (RFC 4340, 19.5) */
+#define CCID_MAX		255
+#define CCID_SLAB_NAME_LENGTH	32
 
 struct tcp_info;
 
@@ -49,8 +51,8 @@ struct ccid_operations {
 	const char		*ccid_name;
 	struct kmem_cache	*ccid_hc_rx_slab,
 				*ccid_hc_tx_slab;
-	char			ccid_hc_rx_slab_name[32];
-	char			ccid_hc_tx_slab_name[32];
+	char			ccid_hc_rx_slab_name[CCID_SLAB_NAME_LENGTH];
+	char			ccid_hc_tx_slab_name[CCID_SLAB_NAME_LENGTH];
 	__u32			ccid_hc_rx_obj_size,
 				ccid_hc_tx_obj_size;
 	/* Interface Routines */

[PATCH 02/86] dccp: revert buggy auto-loading of dccp module

[Gerrit Renker]

This reverts commit (38ff3e6bb987ec583268da8eb22628293095d43b) ("dccp_probe:
Fix module load dependencies between dccp and dccp_probe", from 15 Jan). Not
only does it not work:

% modprobe -v dccp_probe
kernel: [ 1431.442912] sys_init_module: 'dccp_probe'->init suspiciously \
 		      returned 1, it should follow 0/-E convention
kernel: [ 1431.442915] sys_init_module: loading module anyway...


... but it also causes a crash:

% rmmod dccp_probe
kernel: [ 1777.305846] kernel BUG at /usr/src/davem-2.6/mm/slab.c:521!
kernel: [ 1777.305852] invalid opcode: 0000 [#1] SMP
kernel: [ 1777.305861] last sysfs file: /sys/class/power_supply/BAT0/energy_full
kernel: [ 1777.305867] Modules linked in: dccp_probe(-) iwl3945 iwlcore [last unloaded: dccp]
kernel: [ 1777.305883]
kernel: [ 1777.305891] Pid: 12912, comm: rmmod Tainted: G  R        2.6.33-rc5 #6 2008URG/2008URG
kernel: [ 1777.305899] EIP: 0060:[<c01d5e43>] EFLAGS: 00010046 CPU: 1
kernel: [ 1777.305910] EIP is at kfree+0x73/0x150
kernel: [ 1777.305916] EAX: c1678c00 EBX: 00000000 ECX: c01d5e15 EDX: 40080000
kernel: [ 1777.305922] ESI: c015cb9a EDI: 080488a0 EBP: f4ffbf34 ESP: f4ffbf10
kernel: [ 1777.305929]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
kernel: [ 1777.305936] Process rmmod (pid: 12912, ti=f4ffb000 task=f61e8620 task.ti=f4ffb000)

==> After reverting the commit:

% modprobe -v dccp_probe
insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp.ko
insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp_probe.ko

% lsmod
Module                  Size  Used by
dccp_probe              2345  0
dccp                  120233  1 dccp_probe

Previously (during about 4 years of this module's history) there had never
been a problem with the 'silent dependency' that the commit tried to fix:
this dependency is deliberate and required, since dccp_probe performs probing
of dccp connections and hence needs to know about dccp internals.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/probe.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

--- a/net/dccp/probe.c
+++ b/net/dccp/probe.c
@@ -161,8 +161,7 @@ static __init int dccpprobe_init(void)
 	if (!proc_net_fops_create(&init_net, procname, S_IRUSR, &dccpprobe_fops))
 		goto err0;
 
-	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
-					"dccp");
+	ret = register_jprobe(&dccp_send_probe);
 	if (ret)
 		goto err1;
 

Re: [PATCH 02/86] dccp: revert buggy auto-loading of dccp module

[Neil Horman]

On Mon, Feb 01, 2010 at 07:18:07AM +0100, Gerrit Renker wrote:
> This reverts commit (38ff3e6bb987ec583268da8eb22628293095d43b) ("dccp_probe:
> Fix module load dependencies between dccp and dccp_probe", from 15 Jan). Not
> only does it not work:
> 
> % modprobe -v dccp_probe
> kernel: [ 1431.442912] sys_init_module: 'dccp_probe'->init suspiciously \
>  		      returned 1, it should follow 0/-E convention
> kernel: [ 1431.442915] sys_init_module: loading module anyway...
> 
> 
> ... but it also causes a crash:
> 
> % rmmod dccp_probe
> kernel: [ 1777.305846] kernel BUG at /usr/src/davem-2.6/mm/slab.c:521!
> kernel: [ 1777.305852] invalid opcode: 0000 [#1] SMP
> kernel: [ 1777.305861] last sysfs file: /sys/class/power_supply/BAT0/energy_full
> kernel: [ 1777.305867] Modules linked in: dccp_probe(-) iwl3945 iwlcore [last unloaded: dccp]
> kernel: [ 1777.305883]
> kernel: [ 1777.305891] Pid: 12912, comm: rmmod Tainted: G  R        2.6.33-rc5 #6 2008URG/2008URG
> kernel: [ 1777.305899] EIP: 0060:[<c01d5e43>] EFLAGS: 00010046 CPU: 1
> kernel: [ 1777.305910] EIP is at kfree+0x73/0x150
> kernel: [ 1777.305916] EAX: c1678c00 EBX: 00000000 ECX: c01d5e15 EDX: 40080000
> kernel: [ 1777.305922] ESI: c015cb9a EDI: 080488a0 EBP: f4ffbf34 ESP: f4ffbf10
> kernel: [ 1777.305929]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> kernel: [ 1777.305936] Process rmmod (pid: 12912, ti=f4ffb000 task=f61e8620 task.ti=f4ffb000)
> 
> ==> After reverting the commit:
> 
> % modprobe -v dccp_probe
> insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp.ko
> insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp_probe.ko
> 
> % lsmod
> Module                  Size  Used by
> dccp_probe              2345  0
> dccp                  120233  1 dccp_probe
> 
> Previously (during about 4 years of this module's history) there had never
> been a problem with the 'silent dependency' that the commit tried to fix:
> this dependency is deliberate and required, since dccp_probe performs probing
> of dccp connections and hence needs to know about dccp internals.
> 
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>

This doesn't make any sense.  Gerrit, you don't understand what the patch was
trying to do.  There is a silent dependency, in that this module requires the
dccp module to be loaded, but the reference to the dccp_send_probe symbol isn't
one that depmod can see.  If you don't load dccp first, dccp_probe fails, why
bother to allow that when try_then_request_module can avoid it?

The problem here is the construction of the first argument,
try_then_request_module should only return valid return codes from the first
argument, and my first argument is malformed.  register_jprobe returns zero on
success, so I need to check its return in the call for 0, in case we need to
trigger the request_module action, but in so doing ret gets the value of
(register_jprobe(&dccp_send_probe) == 0), which will always be 0 or 1.  What we
actually need to do is assign the result of register_jprobe to ret, without the
side effect of the comparison.  I've not tested it, but this should do it,
without re-breaking the silent dependency.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 
 probe.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/net/dccp/probe.c b/net/dccp/probe.c
index bace1d8..a8f5fdf 100644
--- a/net/dccp/probe.c
+++ b/net/dccp/probe.c
@@ -161,7 +161,8 @@ static __init int dccpprobe_init(void)
 	if (!proc_net_fops_create(&init_net, procname, S_IRUSR, &dccpprobe_fops))
 		goto err0;
 
-	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
+	try_then_request_module(
+		((ret = register_jprobe(&dccp_send_probe)) == 0),
 					"dccp");
 	if (ret)
 		goto err1;

Re: [PATCH 02/86] dccp: revert buggy auto-loading of dccp module

[Neil Horman]

On Mon, Feb 01, 2010 at 07:21:19AM -0500, Neil Horman wrote:
> On Mon, Feb 01, 2010 at 07:18:07AM +0100, Gerrit Renker wrote:
> > This reverts commit (38ff3e6bb987ec583268da8eb22628293095d43b) ("dccp_probe:
> > Fix module load dependencies between dccp and dccp_probe", from 15 Jan). Not
> > only does it not work:
> > 
> > % modprobe -v dccp_probe
> > kernel: [ 1431.442912] sys_init_module: 'dccp_probe'->init suspiciously \
> >  		      returned 1, it should follow 0/-E convention
> > kernel: [ 1431.442915] sys_init_module: loading module anyway...
> > 
> > 
> > ... but it also causes a crash:
> > 
> > % rmmod dccp_probe
> > kernel: [ 1777.305846] kernel BUG at /usr/src/davem-2.6/mm/slab.c:521!
> > kernel: [ 1777.305852] invalid opcode: 0000 [#1] SMP
> > kernel: [ 1777.305861] last sysfs file: /sys/class/power_supply/BAT0/energy_full
> > kernel: [ 1777.305867] Modules linked in: dccp_probe(-) iwl3945 iwlcore [last unloaded: dccp]
> > kernel: [ 1777.305883]
> > kernel: [ 1777.305891] Pid: 12912, comm: rmmod Tainted: G  R        2.6.33-rc5 #6 2008URG/2008URG
> > kernel: [ 1777.305899] EIP: 0060:[<c01d5e43>] EFLAGS: 00010046 CPU: 1
> > kernel: [ 1777.305910] EIP is at kfree+0x73/0x150
> > kernel: [ 1777.305916] EAX: c1678c00 EBX: 00000000 ECX: c01d5e15 EDX: 40080000
> > kernel: [ 1777.305922] ESI: c015cb9a EDI: 080488a0 EBP: f4ffbf34 ESP: f4ffbf10
> > kernel: [ 1777.305929]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> > kernel: [ 1777.305936] Process rmmod (pid: 12912, ti=f4ffb000 task=f61e8620 task.ti=f4ffb000)
> > 
> > ==> After reverting the commit:
> > 
> > % modprobe -v dccp_probe
> > insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp.ko
> > insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp_probe.ko
> > 
> > % lsmod
> > Module                  Size  Used by
> > dccp_probe              2345  0
> > dccp                  120233  1 dccp_probe
> > 
> > Previously (during about 4 years of this module's history) there had never
> > been a problem with the 'silent dependency' that the commit tried to fix:
> > this dependency is deliberate and required, since dccp_probe performs probing
> > of dccp connections and hence needs to know about dccp internals.
> > 
> > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> 
> This doesn't make any sense.  Gerrit, you don't understand what the patch was
> trying to do.  There is a silent dependency, in that this module requires the
> dccp module to be loaded, but the reference to the dccp_send_probe symbol isn't
> one that depmod can see.  If you don't load dccp first, dccp_probe fails, why
> bother to allow that when try_then_request_module can avoid it?
> 
> The problem here is the construction of the first argument,
> try_then_request_module should only return valid return codes from the first
> argument, and my first argument is malformed.  register_jprobe returns zero on
> success, so I need to check its return in the call for 0, in case we need to
> trigger the request_module action, but in so doing ret gets the value of
> (register_jprobe(&dccp_send_probe) == 0), which will always be 0 or 1.  What we
> actually need to do is assign the result of register_jprobe to ret, without the
> side effect of the comparison.  I've not tested it, but this should do it,
> without re-breaking the silent dependency.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> 
>  
>  probe.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/net/dccp/probe.c b/net/dccp/probe.c
> index bace1d8..a8f5fdf 100644
> --- a/net/dccp/probe.c
> +++ b/net/dccp/probe.c
> @@ -161,7 +161,8 @@ static __init int dccpprobe_init(void)
>  	if (!proc_net_fops_create(&init_net, procname, S_IRUSR, &dccpprobe_fops))
>  		goto err0;
>  
> -	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
> +	try_then_request_module(
> +		((ret = register_jprobe(&dccp_send_probe)) == 0),
>  					"dccp");
>  	if (ret)
>  		goto err1;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Gerrit, any thoughts here?
Thanks & Regards
Neil

Re: [PATCH 02/86] dccp: revert buggy auto-loading of dccp module

[gerrit]

> --- a/net/dccp/probe.c
> +++ b/net/dccp/probe.c
> @@ -161,7 +161,8 @@ static __init int dccpprobe_init(void)
>  	if (!proc_net_fops_create(&init_net, procname, S_IRUSR,
> &dccpprobe_fops))
>  		goto err0;
>
> -	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
> +	try_then_request_module(
> +		((ret = register_jprobe(&dccp_send_probe)) == 0),
>  					"dccp");
>  	if (ret)
>  		goto err1;

Apologies for the late response -- delays are sometimes possible due to
day job. The only problem that I had with this patch was that it was
apparently not tested, causing the described problems.


> I've not tested it, but this should do it,
> without re-breaking the silent dependency.

I will test it and get back to you until tomorrow morning (GMT),
if it works I'll also push it out in the test tree.

Thanks a lot for getting back and devising a different route.

Re: [PATCH 02/86] dccp: revert buggy auto-loading of dccp module

[Neil Horman]

On Tue, Feb 02, 2010 at 03:06:15PM -0000, gerrit@erg.abdn.ac.uk wrote:
> > --- a/net/dccp/probe.c
> > +++ b/net/dccp/probe.c
> > @@ -161,7 +161,8 @@ static __init int dccpprobe_init(void)
> >  	if (!proc_net_fops_create(&init_net, procname, S_IRUSR,
> > &dccpprobe_fops))
> >  		goto err0;
> >
> > -	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
> > +	try_then_request_module(
> > +		((ret = register_jprobe(&dccp_send_probe)) == 0),
> >  					"dccp");
> >  	if (ret)
> >  		goto err1;
> 
> Apologies for the late response -- delays are sometimes possible due to
> day job. The only problem that I had with this patch was that it was
> apparently not tested, causing the described problems.
> 
> 
> > I've not tested it, but this should do it,
> > without re-breaking the silent dependency.
> 
> I will test it and get back to you until tomorrow morning (GMT),
> if it works I'll also push it out in the test tree.
> 
> Thanks a lot for getting back and devising a different route.
> 
Thanks, and apologies for being brusque.  I just didn't want to fix what is
clearly a problem in what you describe by rebreaking a previously seen problem.
I expect this should work just fine.  Let me know if it doesn't and I'll
resurrect my test bed here and take a look
Neil

> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 1/1] dccp: fix auto-loading of dccp(_probe)

[Gerrit Renker]

Hi Neil,

I have tested your patch, it works fine thanks to the repeated execution of the macro
argument. I have reformatted it, removing the inner parentheses, which I think is
justified since macros copy their arguments in full. Also converted your email to a
drafted commit message. Due to this, have not copied your signed-off -- if you are
ok, please add, this should be submitted asap.

>>>>>>>>>>>>>>>>>>>>>>>>>>> Patch v2 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
dccp: fix loading of dccp_probe

This fixes commit (38ff3e6bb987ec583268da8eb22628293095d43b) ("dccp_probe:
Fix module load dependencies between dccp and dccp_probe", from 15 Jan).

It fixes the construction of the first argument of try_then_request_module(),
where only valid return codes from the first argument should be returned.

What we do now is assign the result of register_jprobe() to ret, without
the side effect of the comparison.

Acked-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/probe.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/net/dccp/probe.c
+++ b/net/dccp/probe.c
@@ -161,8 +161,8 @@ static __init int dccpprobe_init(void)
 	if (!proc_net_fops_create(&init_net, procname, S_IRUSR, &dccpprobe_fops))
 		goto err0;
 
-	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
-					"dccp");
+	try_then_request_module((ret = register_jprobe(&dccp_send_probe)) == 0,
+				"dccp");
 	if (ret)
 		goto err1;
 

Re: [PATCH v2 1/1] dccp: fix auto-loading of dccp(_probe)

[Neil Horman]

On Wed, Feb 03, 2010 at 07:16:56AM +0100, Gerrit Renker wrote:
> Hi Neil,
> 
> I have tested your patch, it works fine thanks to the repeated execution of the macro
> argument. I have reformatted it, removing the inner parentheses, which I think is
> justified since macros copy their arguments in full. Also converted your email to a
> drafted commit message. Due to this, have not copied your signed-off -- if you are
> ok, please add, this should be submitted asap.
> 
Understood, and thank you.  This looks fine to me

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

> >>>>>>>>>>>>>>>>>>>>>>>>>>> Patch v2 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> dccp: fix loading of dccp_probe
> 
> This fixes commit (38ff3e6bb987ec583268da8eb22628293095d43b) ("dccp_probe:
> Fix module load dependencies between dccp and dccp_probe", from 15 Jan).
> 
> It fixes the construction of the first argument of try_then_request_module(),
> where only valid return codes from the first argument should be returned.
> 
> What we do now is assign the result of register_jprobe() to ret, without
> the side effect of the comparison.
> 
> Acked-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
>  net/dccp/probe.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/net/dccp/probe.c
> +++ b/net/dccp/probe.c
> @@ -161,8 +161,8 @@ static __init int dccpprobe_init(void)
>  	if (!proc_net_fops_create(&init_net, procname, S_IRUSR, &dccpprobe_fops))
>  		goto err0;
>  
> -	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
> -					"dccp");
> +	try_then_request_module((ret = register_jprobe(&dccp_send_probe)) == 0,
> +				"dccp");
>  	if (ret)
>  		goto err1;
>  
> 

Re: [PATCH v2 1/1] dccp: fix auto-loading of dccp(_probe)

[David Miller]

From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 3 Feb 2010 06:58:40 -0500

> On Wed, Feb 03, 2010 at 07:16:56AM +0100, Gerrit Renker wrote:
>> Hi Neil,
>> 
>> I have tested your patch, it works fine thanks to the repeated execution of the macro
>> argument. I have reformatted it, removing the inner parentheses, which I think is
>> justified since macros copy their arguments in full. Also converted your email to a
>> drafted commit message. Due to this, have not copied your signed-off -- if you are
>> ok, please add, this should be submitted asap.
>> 
> Understood, and thank you.  This looks fine to me
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Applied, thanks.

Re: [PATCH 01/86] dccp: fix bug in cache allocation

[Neil Horman]

On Mon, Feb 01, 2010 at 07:18:06AM +0100, Gerrit Renker wrote:
> This fixes a bug introduced in commit de4ef86cfce60d2250111f34f8a084e769f23b16
> ("dccp: fix dccp rmmod when kernel configured to use slub", 17 Jan): the
> vsnprintf used sizeof(slab_name_fmt), which became truncated to 4 bytes, since
> slab_name_fmt is now a 4-byte pointer and no longer a 32-character array.
> 
> This lead to error messages such as
>  FATAL: Error inserting dccp: No buffer space available
> 
>  >> kernel: [ 1456.341501] kmem_cache_create: duplicate cache cci
> generated due to the truncation after the 3rrdc character.
> 
> Fixed for the moment by introducing a symbolic constant. Tested to fix the bug.
> 
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>

These changes make sense, sorry for not seeing that earlier.  Thanks!

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> 

Re: [PATCH 01/86] dccp: fix bug in cache allocation

[Gerrit Renker]

| These changes make sense, sorry for not seeing that earlier.  Thanks!
| Acked-by: Neil Horman <nhorman@tuxdriver.com>  
Thanks a lot for the quick response, I was just in the middle of composing
a message to re-send the patches (the count '0{1,2}/86' was wrong).

Apologies for forgetting to CC:, thanks for catching this.

[BUG-FIX v2][PATCH 0/2]: recent dccp fixes

[Gerrit Renker]

Hi Dave,

as per your request, I am resending this CC:-ed to Neil (who already acked
the first patch), and also adding Arnaldo.

These are two fixes for recent dccp commits which addressed issues, but
introduced bugs which I discovered when rebasing the test tree last week.

Patch #1: fixes a bug resulting from truncating cache name length.
Patch #2: reverts a commit which introduced a module-loading bug.

This being bugs, would you please consider this for stable too.

The test-tree also has been fixed accordingly,
http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=shortlog;h=ready    

[Bug-Fix][PATCH 1/2] dccp: fix bug in cache allocation

[Gerrit Renker]

This fixes a bug introduced in commit de4ef86cfce60d2250111f34f8a084e769f23b16
("dccp: fix dccp rmmod when kernel configured to use slub", 17 Jan): the
vsnprintf used sizeof(slab_name_fmt), which became truncated to 4 bytes, since
slab_name_fmt is now a 4-byte pointer and no longer a 32-character array.

This lead to error messages such as
 FATAL: Error inserting dccp: No buffer space available

 >> kernel: [ 1456.341501] kmem_cache_create: duplicate cache cci
generated due to the truncation after the 3rd character.

Fixed for the moment by introducing a symbolic constant. Tested to fix the bug.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Acked-by: Neil Horman <nhorman@tuxdriver.com> 
---
 net/dccp/ccid.c |    2 +-
 net/dccp/ccid.h |    8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

--- a/net/dccp/ccid.c
+++ b/net/dccp/ccid.c
@@ -83,7 +83,7 @@ static struct kmem_cache *ccid_kmem_cache_create(int obj_size, char *slab_name_f
 	va_list args;
 
 	va_start(args, fmt);
-	vsnprintf(slab_name_fmt, sizeof(slab_name_fmt), fmt, args);
+	vsnprintf(slab_name_fmt, CCID_SLAB_NAME_LENGTH, fmt, args);
 	va_end(args);
 
 	slab = kmem_cache_create(slab_name_fmt, sizeof(struct ccid) + obj_size, 0,
--- a/net/dccp/ccid.h
+++ b/net/dccp/ccid.h
@@ -19,7 +19,9 @@
 #include <linux/list.h>
 #include <linux/module.h>
 
-#define CCID_MAX 255
+/* maximum value for a CCID (RFC 4340, 19.5) */
+#define CCID_MAX		255
+#define CCID_SLAB_NAME_LENGTH	32
 
 struct tcp_info;
 
@@ -49,8 +51,8 @@ struct ccid_operations {
 	const char		*ccid_name;
 	struct kmem_cache	*ccid_hc_rx_slab,
 				*ccid_hc_tx_slab;
-	char			ccid_hc_rx_slab_name[32];
-	char			ccid_hc_tx_slab_name[32];
+	char			ccid_hc_rx_slab_name[CCID_SLAB_NAME_LENGTH];
+	char			ccid_hc_tx_slab_name[CCID_SLAB_NAME_LENGTH];
 	__u32			ccid_hc_rx_obj_size,
 				ccid_hc_tx_obj_size;
 	/* Interface Routines */

[Bug-Fix][PATCH 2/2] dccp: revert buggy auto-loading of dccp module

[Gerrit Renker]

This reverts commit (38ff3e6bb987ec583268da8eb22628293095d43b) ("dccp_probe:
Fix module load dependencies between dccp and dccp_probe", from 15 Jan). Not
only does it not work:

% modprobe -v dccp_probe
kernel: [ 1431.442912] sys_init_module: 'dccp_probe'->init suspiciously \
 		      returned 1, it should follow 0/-E convention
kernel: [ 1431.442915] sys_init_module: loading module anyway...


... but it also causes a crash:

% rmmod dccp_probe
kernel: [ 1777.305846] kernel BUG at /usr/src/davem-2.6/mm/slab.c:521!
kernel: [ 1777.305852] invalid opcode: 0000 [#1] SMP
kernel: [ 1777.305861] last sysfs file: /sys/class/power_supply/BAT0/energy_full
kernel: [ 1777.305867] Modules linked in: dccp_probe(-) iwl3945 iwlcore [last unloaded: dccp]
kernel: [ 1777.305883]
kernel: [ 1777.305891] Pid: 12912, comm: rmmod Tainted: G  R        2.6.33-rc5 #6 2008URG/2008URG
kernel: [ 1777.305899] EIP: 0060:[<c01d5e43>] EFLAGS: 00010046 CPU: 1
kernel: [ 1777.305910] EIP is at kfree+0x73/0x150
kernel: [ 1777.305916] EAX: c1678c00 EBX: 00000000 ECX: c01d5e15 EDX: 40080000
kernel: [ 1777.305922] ESI: c015cb9a EDI: 080488a0 EBP: f4ffbf34 ESP: f4ffbf10
kernel: [ 1777.305929]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
kernel: [ 1777.305936] Process rmmod (pid: 12912, ti=f4ffb000 task=f61e8620 task.ti=f4ffb000)

==> After reverting the commit:

% modprobe -v dccp_probe
insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp.ko
insmod /lib/modules/2.6.33-rc5/kernel/net/dccp/dccp_probe.ko

% lsmod
Module                  Size  Used by
dccp_probe              2345  0
dccp                  120233  1 dccp_probe

Previously (during about 4 years of this module's history) there had never
been a problem with the 'silent dependency' that the commit tried to fix:
this dependency is deliberate and required, since dccp_probe performs probing
of dccp connections and hence needs to know about dccp internals.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/probe.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

--- a/net/dccp/probe.c
+++ b/net/dccp/probe.c
@@ -161,8 +161,7 @@ static __init int dccpprobe_init(void)
 	if (!proc_net_fops_create(&init_net, procname, S_IRUSR, &dccpprobe_fops))
 		goto err0;
 
-	ret = try_then_request_module((register_jprobe(&dccp_send_probe) == 0),
-					"dccp");
+	ret = register_jprobe(&dccp_send_probe);
 	if (ret)
 		goto err1;
 

Re: [PATCH 01/86] dccp: fix bug in cache allocation

[David Miller]

From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 1 Feb 2010 06:56:38 -0500

> On Mon, Feb 01, 2010 at 07:18:06AM +0100, Gerrit Renker wrote:
>> This fixes a bug introduced in commit de4ef86cfce60d2250111f34f8a084e769f23b16
>> ("dccp: fix dccp rmmod when kernel configured to use slub", 17 Jan): the
>> vsnprintf used sizeof(slab_name_fmt), which became truncated to 4 bytes, since
>> slab_name_fmt is now a 4-byte pointer and no longer a 32-character array.
>> 
>> This lead to error messages such as
>>  FATAL: Error inserting dccp: No buffer space available
>> 
>>  >> kernel: [ 1456.341501] kmem_cache_create: duplicate cache cci
>> generated due to the truncation after the 3rrdc character.
>> 
>> Fixed for the moment by introducing a symbolic constant. Tested to fix the bug.
>> 
>> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> 
> These changes make sense, sorry for not seeing that earlier.  Thanks!
> 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Applied.

Re: [BUG-FIX][PATCH 0/2]: recent dccp fixes

[David Miller]

From: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Date: Mon,  1 Feb 2010 07:18:05 +0100

> In January there were two dccp commits introducing bugs. Having
> been guilty of ack-ing them, please find below patches to fix
> them.
> 
> Patch #1: fixes a bug resulting from truncating cache name length.
> Patch #2: reverts a commit which introduced a module-loading bug.
> 
> This being bugs, would you please consider this for stable too.
> 
> The test-tree also has been fixed accordingly,
> http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=shortlog;h=ready    

Gerrit can you at least CC: the author of the changes
you are reverting?