Fri 28 Aug 2015 12:37:26 PM UTC, comment #13:
Thanks :-)
|
Fri 28 Aug 2015 12:33:17 PM UTC, comment #12:
Erm. Nevermind, just got fooled by a typo in the assert comment and syntax hilighting telling me the assert was still used. Anyway, typo fixed IP6_FRAG_COPYHEADER -> IPV6_FRAG_COPYHEADER.
|
Thu 27 Aug 2015 09:37:24 PM UTC, comment #11:
I failed to see how the assert works, IP6_FRAG_COPYHEADER does not change sizeof(struct ip6_reass_helper) nor IP6_FRAG_HLEN right ? So the assert is always triggered isn't it ?
Sylvain
|
Thu 27 Aug 2015 07:45:41 PM UTC, comment #10:
Honestly, I don't want to fix this for windows and linux only. I've instead added a sanity check to ip6_reass_tmr(), so you don't actually need to receive a fragmented packet to trigger the assert. Any good compiler should optimize this check to nothing on 32-or-less-bit platforms.
|
Wed 26 Aug 2015 09:51:29 PM UTC, comment #9:
#define IPV6_FRAG_COPYHEADER (defined(_LP64_) || defined(_WIN64))
might be fine as well, and a little bit more generic.
|
Wed 26 Aug 2015 09:43:24 PM UTC, comment #8:
Indeed, I failed to add that any usage of IP6_FRAG_COPYHEADER should be rewritten using if() { }.
If we want to keep MACRO, we can still use something like:
#define IPV6_FRAG_COPYHEADER (defined(_x86_64_) && _x86_64_ || defined(_M_X64) && _M_X64)
Which should do the job at least for gcc and msvc.
|
Wed 26 Aug 2015 09:24:16 PM UTC, comment #7:
Preprocessor doesn't do sizeof().
|
Wed 26 Aug 2015 09:16:58 PM UTC, comment #6:
What about adding the following in opt.h (instead of living in ip6_frag.h):
/** IP6_FRAG_COPYHEADER==1: When enabled, the IPv6 header is copied instead of
- referencing it. For platforms where sizeof(void*) > 4, this needs to be
- enabled (to not overwrite part of the data). It gives more room for
- struct ip6_reass_helper */
#ifndef IPV6_FRAG_COPYHEADER
#define IPV6_FRAG_COPYHEADER (sizeof(void*) > 4)
#endif
|
Wed 26 Aug 2015 08:17:19 PM UTC, comment #5:
Fixed with a minimal change:
define IPV6_FRAG_COPYHEADER==1 on these platforms to copy the IPv6 header instead of referencing it, which gives more room for struct ip6_reass_helper
|
Tue 24 Mar 2015 07:06:52 PM UTC, comment #4:
This still should be fixed for 1.5.0
|
Fri 06 Mar 2015 09:16:50 PM UTC, comment #3:
As the patch is only required for >32 bit machines and it's no a trivial one, I'm not applying this without thorough testing.
As you said, this patch is "rewritten assembly code", not just fixing 64-bit compatibility issues. I'll need more time to verify the patch as it is.
I'm attaching an updated diff against 007ab69777219848dc0d205ec9bb9594ec1eb60b to make it easier to reapply it.
(file #33272)
|
Mon 17 Nov 2014 11:43:18 AM UTC, comment #2:
Attached is rewritten reassembly code. The diff corresponds to tree state before 381a7b110a4a611e9956b2c644bc330bacb87073 ("fixed bug #41041 Potential use-after-free in IPv6 reassembly"), which is superceeded by this diff. Typos fixed by the two commits after that are also already fixed in this diff.
(file #32451)
|
Fri 10 Jan 2014 09:09:51 PM UTC, comment #1:
You're right. I've just added an assert should triggers this. However, I don't have an idea how to solve this right now.
|
Fri 27 Dec 2013 01:28:31 AM UTC, original submission:
struct ip6_reass_helper "replaces the Fragment Header in memory in
incoming fragments". Unfortunately, it contains struct pbuf
*next_pbuf pointer, so on 64-bit platforms that struct is larger than
struct ip6_frag_hdr and reassembly code overwrites the first 4 bytes
of each fragment.
|