Bug 17622
| Summary: | snmp trap ALG issue | ||
|---|---|---|---|
| Product: | Networking | Reporter: | clark wang (wtweeker) |
| Component: | IPV4 | Assignee: | Stephen Hemminger (stephen) |
| Status: | RESOLVED OBSOLETE | ||
| Severity: | normal | CC: | alan, wtweeker |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.35.4 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
|
Description
clark wang
2010-09-02 09:25:11 UTC
I have changed this function by refering to other checksum algorithm.
And tested it, it can work. the checksum is correct.
static void fast_csum(__sum16 *csum,
const unsigned char *optr,
const unsigned char *nptr,
int offset)
{
unsigned char s[4];
if (offset & 1) {
s[0] = s[2] = 0;
s[0] = ~s[0]; //this line is add by me
s[1] = ~*optr;
s[3] = *nptr;
} else {
s[1] = s[3] = 0;
s[1] = ~s[1];//this line is add by me
s[0] = ~*optr;
s[2] = *nptr;
}
*csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));
}
(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface). On Thu, 2 Sep 2010 09:25:12 GMT bugzilla-daemon@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=17622 > > Summary: snmp trap ALG issue > Product: Networking > Version: 2.5 > Kernel Version: 2.6.35.4 > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: IPV4 > AssignedTo: shemminger@linux-foundation.org > ReportedBy: wtweeker@163.com > Regression: No > > > Symptom: > SNMP manager can't show trap when SNMP agent set trap message to version1. > > steps to reproduce: > (1)SNMP agent-----linux device(NAT)-----SNMP manager. > (2)Set SNMP agent trap message to version 1, destination IP as SNMP > manger's > IP. > (3)Do some operation to generate trap message, such as make one port of SNMP > agent up and down. But SNMP manger can't accept trap message. > > I have capured the packet by Ethereal software, and check the SNMP trap > packet, > found that the UDP checksum is incorrect. > I think that the function fast_csum()(nf_nat_snmp_basic.c) have some problem. > and > I have changed this function by refering to other checksum algorithm. > And tested it, it can work. the checksum is correct. > > static void fast_csum(__sum16 *csum, > const unsigned char *optr, > const unsigned char *nptr, > int offset) > { > unsigned char s[4]; > > if (offset & 1) { > s[0] = s[2] = 0; > s[0] = ~s[0]; //this line is add by me > s[1] = ~*optr; > s[3] = *nptr; > } else { > s[1] = s[3] = 0; > s[1] = ~s[1];//this line is add by me > s[0] = ~*optr; > s[2] = *nptr; > } > > *csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum))); > } Great. Please prepare a kernel patch as per Documentation/SubmittingPatches and send it via a reply-to-all to this email? Thanks. Dear All
The attachment is the patch of this bug.
Thanks
At 2010-09-15 07:32:08，"Andrew Morton" <akpm@linux-foundation.org> wrote:
>(switched to email. Please respond via emailed reply-to-all, not via the
>bugzilla web interface).
>
>On Thu, 2 Sep 2010 09:25:12 GMT
>bugzilla-daemon@bugzilla.kernel.org wrote:
>
>> https://bugzilla.kernel.org/show_bug.cgi?id=17622
>>
>> Summary: snmp trap ALG issue
>> Product: Networking
>> Version: 2.5
>> Kernel Version: 2.6.35.4
>> Platform: All
>> OS/Version: Linux
>> Tree: Mainline
>> Status: NEW
>> Severity: normal
>> Priority: P1
>> Component: IPV4
>> AssignedTo: shemminger@linux-foundation.org
>> ReportedBy: wtweeker@163.com
>> Regression: No
>>
>>
>> Symptom:
>> SNMP manager can't show trap when SNMP agent set trap message to version1.
>>
>> steps to reproduce:
>> (1)SNMP agent-----linux device(NAT)-----SNMP manager.
>> (2)Set SNMP agent trap message to version 1, destination IP as SNMP
>> manger's
>> IP.
>> (3)Do some operation to generate trap message, such as make one port of SNMP
>> agent up and down. But SNMP manger can't accept trap message.
>>
>> I have capured the packet by Ethereal software, and check the SNMP trap
>> packet,
>> found that the UDP checksum is incorrect.
>> I think that the function fast_csum()(nf_nat_snmp_basic.c) have some
>> problem.
>>
>
>and
>
>
>> I have changed this function by refering to other checksum algorithm.
>> And tested it, it can work. the checksum is correct.
>>
>> static void fast_csum(__sum16 *csum,
>> const unsigned char *optr,
>> const unsigned char *nptr,
>> int offset)
>> {
>> unsigned char s[4];
>>
>> if (offset & 1) {
>> s[0] = s[2] = 0;
>> s[0] = ~s[0]; //this line is add by me
>> s[1] = ~*optr;
>> s[3] = *nptr;
>> } else {
>> s[1] = s[3] = 0;
>> s[1] = ~s[1];//this line is add by me
>> s[0] = ~*optr;
>> s[2] = *nptr;
>> }
>>
>> *csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));
>> }
>
>Great. Please prepare a kernel patch as per
>Documentation/SubmittingPatches and send it via a reply-to-all to this
>email?
>
>Thanks.
Reply-To: shemminger@vyatta.com I think the bug should be fixed by removing the potentially buggy fast_csum() in nf_nat_snmp_basic and just using the existing generic code. The following is compile tested only.. Subject: [PATCH] nf_nat_snmp: use existing checksum update code The fast_csum() in NAT code for processing SNMP trap is buggy (see https://bugzilla.kernel.org/show_bug.cgi?id=17622) Replace it by using the existing checksum replacement code; it means adding a new csum_replace1() inline wrapper. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> --- include/net/checksum.h | 5 +++++ net/ipv4/netfilter/nf_nat_snmp_basic.c | 31 ++----------------------------- 2 files changed, 7 insertions(+), 29 deletions(-) --- a/net/ipv4/netfilter/nf_nat_snmp_basic.c 2010-09-16 22:17:21.660806917 -0700 +++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c 2010-09-16 22:32:52.084075112 -0700 @@ -882,30 +882,6 @@ static unsigned char snmp_request_decode } /* - * Fast checksum update for possibly oddly-aligned UDP byte, from the - * code example in the draft. - */ -static void fast_csum(__sum16 *csum, - const unsigned char *optr, - const unsigned char *nptr, - int offset) -{ - unsigned char s[4]; - - if (offset & 1) { - s[0] = s[2] = 0; - s[1] = ~*optr; - s[3] = *nptr; - } else { - s[1] = s[3] = 0; - s[0] = ~*optr; - s[2] = *nptr; - } - - *csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum))); -} - -/* * Mangle IP address. * - begin points to the start of the snmp messgae * - addr points to the start of the address @@ -924,11 +900,8 @@ static inline void mangle_address(unsign *addr = map->to; /* Update UDP checksum if being used */ - if (*check) { - fast_csum(check, - &map->from, &map->to, addr - begin); - - } + if (*check) + csum_replace1(check, map->from, map->to); if (debug) printk(KERN_DEBUG "bsalg: mapped %pI4 to %pI4\n", --- a/include/net/checksum.h 2010-09-16 22:31:27.524503074 -0700 +++ b/include/net/checksum.h 2010-09-16 22:32:09.934282263 -0700 @@ -106,6 +106,11 @@ static inline void csum_replace2(__sum16 csum_replace4(sum, (__force __be32)from, (__force __be32)to); } +static inline void csum_replace1(__sum16 *sum, __u8 from, __u8 to) +{ + csum_replace4(sum, (__force __be32)from, (__force __be32)to); +} + struct sk_buff; extern void inet_proto_csum_replace4(__sum16 *sum, struct sk_buff *skb, __be32 from, __be32 to, int pseudohdr); Am 17.09.2010 07:39, schrieb Stephen Hemminger:
> nf_nat_snmp: use existing checksum update code
>
> The fast_csum() in NAT code for processing SNMP trap is buggy
> (see https://bugzilla.kernel.org/show_bug.cgi?id=17622)
> Replace it by using the existing checksum replacement code;
> it means adding a new csum_replace1() inline wrapper.
Applied, thanks Stephen.
Reply-To: shemminger@vyatta.com On Fri, 17 Sep 2010 14:08:56 +0200 Patrick McHardy <kaber@trash.net> wrote: > Am 17.09.2010 07:39, schrieb Stephen Hemminger: > > nf_nat_snmp: use existing checksum update code > > > > The fast_csum() in NAT code for processing SNMP trap is buggy > > (see https://bugzilla.kernel.org/show_bug.cgi?id=17622) > > Replace it by using the existing checksum replacement code; > > it means adding a new csum_replace1() inline wrapper. > > Applied, thanks Stephen. As I said in the patch, this was compile tested only. It would be good if the original Bug submitter validated that this fixed the problem. Am 17.09.2010 17:31, schrieb Stephen Hemminger:
> On Fri, 17 Sep 2010 14:08:56 +0200
> Patrick McHardy <kaber@trash.net> wrote:
>
>> Am 17.09.2010 07:39, schrieb Stephen Hemminger:
>>> nf_nat_snmp: use existing checksum update code
>>>
>>> The fast_csum() in NAT code for processing SNMP trap is buggy
>>> (see https://bugzilla.kernel.org/show_bug.cgi?id=17622)
>>> Replace it by using the existing checksum replacement code;
>>> it means adding a new csum_replace1() inline wrapper.
>>
>> Applied, thanks Stephen.
>
> As I said in the patch, this was compile tested only. It would
> be good if the original Bug submitter validated that this
> fixed the problem.
Thanks, that slipped past my eyes. I'll make sure to wait for
confirmation from the reporter or try to test this myself before
sending it upstream.
The way by adding a new csum_replace1() inline wrapper have problem.
And I test it , it can't work. The reason is that it doesn't consider
the value of offset(start addr of snmp message - start addr of agent-addr field)
I provied a solution to resolve this issue, you can refer to it. And this solution was tested by me, it can work well.
--- old\net\ipv4\netfilter\nf_nat_snmp_basic.c Wed Sep 15 09:04:30 2010 UTC
+++ new\net\ipv4\netfilter\nf_nat_snmp_basic.c Wed Sep 15 09:03:43 2010 UTC
@@ -879,10 +879,12 @@
if (offset & 1) {{/*offset is odd numbers, trun a 8-bits into 16-bits by adding the high 8-bits 0x0000*/
s[0] = s[2] = 0;
+ s[0] = ~s[0];
s[1] = ~*optr;
s[3] = *nptr;
} else {{/*offset is even numbers, trun a 8-bits into 16-bits by adding the low 8-bits 0x0000*/
s[1] = s[3] = 0;
+ s[1] = ~s[1];
s[0] = ~*optr;
s[2] = *nptr;
}
|