[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20100428181645.GD4818@hmsreliant.think-freely.org>
Date: Wed, 28 Apr 2010 14:16:45 -0400
From: Neil Horman <nhorman@...driver.com>
To: Vlad Yasevich <vladislav.yasevich@...com>
Cc: sri@...ibm.com, linux-sctp@...r.kernel.org, eteo@...hat.com,
netdev@...r.kernel.org, davem@...emloft.net, security@...nel.org
Subject: Re: [PATCH]: sctp: Fix skb_over_panic resulting from multiple
invalid parameter errors (CVE-2010-1173)
On Wed, Apr 28, 2010 at 01:52:05PM -0400, Vlad Yasevich wrote:
>
>
> Vlad Yasevich wrote:
> >
> > Neil Horman wrote:
> >> On Wed, Apr 28, 2010 at 10:00:37AM -0400, Vlad Yasevich wrote:
> >>> I have this patch and a few others already queued.
> >>>
> >>> I was planning on sending these today for stable.
> >>>
> >>> Here is the full list of stable patches I have:
> >>>
> >>> sctp: Fix oops when sending queued ASCONF chunks
> >>> sctp: fix to calc the INIT/INIT-ACK chunk length correctly is set
> >>> sctp: per_cpu variables should be in bh_disabled section
> >>> sctp: fix potential reference of a freed pointer
> >>> sctp: avoid irq lock inversion while call sk->sk_data_ready()
> >>>
> >>> -vlad
> >>>
> >> Are you sure? this oops looks _very_ simmilar to the INIT/INIT-ACK length
> >> calculation oops described above, but is in fact different, and requires this
> >> patch, from what I can see. The right fix might be in the ASCONF chunk patch
> >> you list above, but I don't see that in your tree at the moment, so I can't be
> >> sure.
> >
> > As I said, I totally goofed when reading the description and I apologize.
> > However, I do one comment regarding the patch.
> >
> > If the bad packet is REALLY long (I mean close to 65K IP limit), then
> > we'll end up allocating a supper huge skb in this case and potentially exceed
> > the IP length limitation. Section 11.4 of rfc 4960 allows us to omit some
> > errors and limit the size of the packet.
> >
> > I would recommend limiting this to MTU worth of potentiall errors. This is
> > on top of what the INIT-ACK is going to carry, so at most we'll sent 2 MTUs
> > worth. That's still a potential by amplification attack, but it's somewhat
> > mitigated.
> >
> > Of course now we have to handle the case of checking for space before adding
> > an error cause. :)
> >
>
> Hi Neil
>
> I am also not crazy about the pre-allocation scheme. In the case where you have
> say 100 parameters that are all 'skip' parameters, you'd end up pre-allocating a
> huge buffer for absolutely nothing.
>
Would have been nice if you'd made your opinion known 4 hours ago when I was
testing version 2 of this. :)
> This is another point toward a fixed error chunk size and let parameter
> processing allocate it when it reaches a parameter that needs an error.
>
Hmm, ok, what would you say to a pathmtu sized chunk allocation in parameter
processing that drops errors beyond its capacity
Neil
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists