- User Since
- Sep 8 2015, 10:30 PM (149 w, 2 d)
Good point on sub structures.
If the cb_destroy virtual function shouldn't null the pointer, I assert that it shouldn't free it either.
Mon, Jul 16
Sun, Jul 15
fix a potential use after free in access to inp_options
Mon, Jul 9
use correct constants
Sun, Jul 8
I agree that using less macros didn't particularly help with readability. But there are in fact two problems with the current code. The issue you pointed out with regard to protecting a user space controlled kernel memory allocation, and secondly, properly detecting and recovering from the requested data length changing when the in6p is reacquired.
Fix GET_PKTOPT_VAR bugs with memory allocation.
Wed, Jul 4
@melifaro this is probably what you were thinking?
refactor GET_PKTOPT_VAR into a function
Tue, Jul 3
I believe I have addressed all concerns, primarily by more fully understanding the functions that were interrelated and realizing that there wasn't anywhere near as much to fix in the first place. The earlier revisions of this patch were far reaching, but now the fixes are just internal to tcp_ipsec_pcbctl()
@melifaro I was not able to refactor into functions but I was able to refactor into something easier to follow and more correct.
update GET_PKTOPT_VAR to properly check for pktopt change after dropping lock
Tue, Jun 26
fix scope of options to match that of len
Jun 14 2018
Looks good to me.
May 3 2018
I agree that having the default stack as a module would be useful for A/B testing the default stack and pushing out fixes without rebooting, and is sufficiently valuable for inclusion into FreeBSD.
Mar 26 2018
Any radical departure from the current behavior of basically creating and copying struct tcp_info to user space would require a different getsockopt() optname at a bare minimum, and we'd still have to support the current TCP_INFO getsockopt() anyway.
Mar 23 2018
malloc and ip options are a bigger problem, going back to the drawing board.
@melifaro I'm thinking about how to make GET_PKTOPT_VAR a function or perhaps... how to make it much smaller and call a function for the bulk of the logic.
I will start a new review to address feedback from @melifaro, this one is broken.
fix malloc length in ip6_output.c:GET_PKTOPT_VAR
Mar 22 2018
use long for len passed to malloc in ip_ctloutput() for getting IP_RETOPTS
fix len passed to malloc in ip_ctloutput() for getting IP_RETOPTS
Mar 20 2018
Mar 19 2018
Mar 10 2018
fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl()
Mar 9 2018
fix IPv6 IPSEC_PCBCTL locking to be consistent with IPv4
Mar 8 2018
I am abandoning this in favor of 6 smaller reviews, per @rwatson's general advice to break up the patch.
Mar 7 2018
Feb 13 2018
use flag instead of cached value for old max_cwnd tracking in cubic
Feb 11 2018
off by one with overflow threshold in cubic_cwnd
Feb 10 2018
Fix underflow and overflow conditions in the cubic cc
Feb 9 2018
Feb 8 2018
Feb 5 2018
Jan 31 2018
Dec 7 2017
Fix memory leak in ip_ctloutput; add documentation to pcbctl function.
Missed INP_WUNLOCK() in ipv6 setsockopt() processing
Oct 27 2017
remove in_pcbref in ctl functions
Oct 26 2017
It looks like a socket is guaranteed to have a single defined inp that is valid (specifically, not free'd) from the time socket or accept is called all the way to the time that close is called.
allow for inp / in6p being null in certain cases within ip_ctl functions
It looks like there might be cases in ip_ctloutput, ip6_ctloutput, rip_ctloutput, and ip6_raw_ctloutput where the inp/in6p being null is ok.
update outstanding issues
Oct 13 2017
Oct 10 2017
Sounds good, I'll abandon this.
No problem, thanks :)
Ok, Sean just reverted the previous patch, and this patch is based on the new HEAD.
rebase patch to new HEAD
We'll probably be abandoning this review in favor of keeping the current behavior and ensuring sendfile behaves the same as send and write.
Oct 9 2017
Gleb wants us to hold off on this for the moment, with the first step of making sendfile() work like current send() / write() paths.
Oct 5 2017
- Consistently handle locking in tcp_ipsec_pcbctl(), as well as use rlocks for reads, and wlocks for writes.
Oct 4 2017
This issue was discovered by rare memory corruption when calling ip6_optlen(). The likely culprit was unlocked manipulation of in6p->in6p_outputopts.
rename OPTSET2292_REQUIRED to OPTSET2282_EXCLUSIVE
Oct 3 2017
Using packetdrill with this https://github.com/nplab/packetdrill/pull/137
Oct 2 2017
Abandoning this due to insufficient proof of a problem.