All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Holler <holler@ahsoftware.de>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: linux-kernel@vger.kernel.org, David Miller <davem@davemloft.net>,
	Herbert Xu <herbert@gondor.hengli.com.au>,
	netdev@vger.kernel.org
Subject: Re: bridge/netfilter: regression in 2.6.39.1
Date: Fri, 03 Jun 2011 21:42:47 +0200	[thread overview]
Message-ID: <4DE93937.7070305@ahsoftware.de> (raw)
In-Reply-To: <1307129642.2600.2.camel@edumazet-laptop>

Am 03.06.2011 21:34, schrieb Eric Dumazet:
> Le vendredi 03 juin 2011 à 21:21 +0200, Alexander Holler a écrit :
>> Hello,
>>
>> I'm getting a oops in the bridge code in br_change_mtu() with 2.6.39.1.
>> The patch below seems to fix that.
>>
>> I'm not sure about the usage of dst_cow_metrics_generic() in
>> fake_dst_ops, but after having a quick look at it seems to be ok to use
>> that here.
>>
>> Regards,
>>
>> Alexander
>>
>> -----
>>   From 3c1d5951af73389798afeea672ec224e195b8e8d Mon Sep 17 00:00:00 2001
>> From: Alexander Holler<holler@ahsoftware.de>
>> Date: Fri, 3 Jun 2011 20:43:06 +0200
>> Subject: [PATCH] bridge: add dst_cow_metrics_generic to fake_dst_ops
>>
>> Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
>> should prevent, it introduces NULL a dereference.
>>
>
> I cant find this commit in known trees. Could you give the real commit
> id and its title ?
>
>> The above commit uses dst_init_metrics() which sets the metrics as
>> read only. As result br_change_mtu() dies in dst_metric_set()
>> which calls dst_metrics_write_ptr() which calls
>> dst->ops->cow_metrics() if the metrics are read only.
>> ---
>>    net/bridge/br_netfilter.c |    1 +
>>    1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>> index 5f9c091..de982a1 100644
>> --- a/net/bridge/br_netfilter.c
>> +++ b/net/bridge/br_netfilter.c
>> @@ -107,6 +107,7 @@ static void fake_update_pmtu(struct dst_entry *dst,
>> u32 mtu)
>>    static struct dst_ops fake_dst_ops = {
>>           .family =               AF_INET,
>>           .protocol =             cpu_to_be16(ETH_P_IP),
>> +       .cow_metrics =          dst_cow_metrics_generic,
>>           .update_pmtu =          fake_update_pmtu,
>>    };
>>
>
> Your patch is mangled (white spaces instead of tabulations)

The patch had a tab, so either c&p failed or something else removed the 
tab. Maybe Thunderbird, don't know. Normally I'm using git send-email.

If someone gives a feedback about the content and not the style, I'm 
willing to send a nice patch which too includes the forgotten Signed-off-by.

Regards,

Alexander


WARNING: multiple messages have this Message-ID (diff)
From: Alexander Holler <holler@ahsoftware.de>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: linux-kernel@vger.kernel.org, David Miller <davem@davemloft.net>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	netdev@vger.kernel.org
Subject: Re: bridge/netfilter: regression in 2.6.39.1
Date: Fri, 03 Jun 2011 21:42:47 +0200	[thread overview]
Message-ID: <4DE93937.7070305@ahsoftware.de> (raw)
In-Reply-To: <1307129642.2600.2.camel@edumazet-laptop>

Am 03.06.2011 21:34, schrieb Eric Dumazet:
> Le vendredi 03 juin 2011 à 21:21 +0200, Alexander Holler a écrit :
>> Hello,
>>
>> I'm getting a oops in the bridge code in br_change_mtu() with 2.6.39.1.
>> The patch below seems to fix that.
>>
>> I'm not sure about the usage of dst_cow_metrics_generic() in
>> fake_dst_ops, but after having a quick look at it seems to be ok to use
>> that here.
>>
>> Regards,
>>
>> Alexander
>>
>> -----
>>   From 3c1d5951af73389798afeea672ec224e195b8e8d Mon Sep 17 00:00:00 2001
>> From: Alexander Holler<holler@ahsoftware.de>
>> Date: Fri, 3 Jun 2011 20:43:06 +0200
>> Subject: [PATCH] bridge: add dst_cow_metrics_generic to fake_dst_ops
>>
>> Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
>> should prevent, it introduces NULL a dereference.
>>
>
> I cant find this commit in known trees. Could you give the real commit
> id and its title ?
>
>> The above commit uses dst_init_metrics() which sets the metrics as
>> read only. As result br_change_mtu() dies in dst_metric_set()
>> which calls dst_metrics_write_ptr() which calls
>> dst->ops->cow_metrics() if the metrics are read only.
>> ---
>>    net/bridge/br_netfilter.c |    1 +
>>    1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>> index 5f9c091..de982a1 100644
>> --- a/net/bridge/br_netfilter.c
>> +++ b/net/bridge/br_netfilter.c
>> @@ -107,6 +107,7 @@ static void fake_update_pmtu(struct dst_entry *dst,
>> u32 mtu)
>>    static struct dst_ops fake_dst_ops = {
>>           .family =               AF_INET,
>>           .protocol =             cpu_to_be16(ETH_P_IP),
>> +       .cow_metrics =          dst_cow_metrics_generic,
>>           .update_pmtu =          fake_update_pmtu,
>>    };
>>
>
> Your patch is mangled (white spaces instead of tabulations)

The patch had a tab, so either c&p failed or something else removed the 
tab. Maybe Thunderbird, don't know. Normally I'm using git send-email.

If someone gives a feedback about the content and not the style, I'm 
willing to send a nice patch which too includes the forgotten Signed-off-by.

Regards,

Alexander

  reply	other threads:[~2011-06-03 19:43 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-03 19:21 bridge/netfilter: regression in 2.6.39.1 Alexander Holler
2011-06-03 19:21 ` Alexander Holler
2011-06-03 19:34 ` Eric Dumazet
2011-06-03 19:34   ` Eric Dumazet
2011-06-03 19:42   ` Alexander Holler [this message]
2011-06-03 19:42     ` Alexander Holler
2011-06-03 19:51   ` Alexander Holler
2011-06-03 19:51     ` Alexander Holler
2011-06-03 19:55     ` Eric Dumazet
2011-06-03 19:55       ` Eric Dumazet
2011-06-04 12:04       ` Alexander Holler
2011-06-04 12:04         ` Alexander Holler
2011-06-06  6:57         ` Alexander Holler
2011-06-06  6:57           ` Alexander Holler
2011-06-03 22:31   ` Ben Hutchings
2011-06-03 22:31     ` Ben Hutchings
2011-06-06 11:15 ` Neil Horman
2011-06-06 11:15   ` Neil Horman
2011-06-06 11:48   ` Alexander Holler
2011-06-06 11:48     ` Alexander Holler
2011-06-06 12:12     ` Eric Dumazet
2011-06-06 12:12       ` Eric Dumazet
2011-06-06 12:49       ` Alexander Holler
2011-06-06 12:49         ` Alexander Holler
2011-06-06 13:13         ` Neil Horman
2011-06-06 13:13           ` Neil Horman
2011-06-06 13:18           ` Eric Dumazet
2011-06-06 13:18             ` Eric Dumazet
2011-06-06 13:16         ` Eric Dumazet
2011-06-06 13:16           ` Eric Dumazet
2011-06-06 13:29           ` Alexander Holler
2011-06-06 13:29             ` Alexander Holler
2011-06-06 14:26             ` Eric Dumazet
2011-06-06 14:26               ` Eric Dumazet
2011-06-06 15:32               ` Neil Horman
2011-06-06 15:32                 ` Neil Horman
2011-06-06 16:11                 ` Eric Dumazet
2011-06-06 16:11                   ` Eric Dumazet
2011-06-06 17:07                   ` Neil Horman
2011-06-06 17:07                     ` Neil Horman
2011-06-07  7:52               ` David Miller
2011-06-06 13:09       ` Eric Dumazet
2011-06-06 13:09         ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DE93937.7070305@ahsoftware.de \
    --to=holler@ahsoftware.de \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=herbert@gondor.hengli.com.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.