This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
The attached patches fix a bug in the net/bsd_tcpip stack, where, when a tcp connection is closed before the user code accepts it, the associated socket is not freed. In our case, this occurred when a TCP connection is made to the unit and immediately reset: Foreign host eCos host -> TCP SYN -> <- TCP SYN,ACK <- -> TCP ACK -> -> TCP RST -> Which occurs during a tcp connect scan or for some management software probing whether the unit is alive. The nmap command: nmap -n -sT -p80 10.0.0.150 where '80' is an open TCP port and '10.0.0.150' is the IP address of the unit, will cause this packet sequence; and, if the RST packet is processed before the 'accept()' call occurs, accept() returns an error but the socket is never freed. On receiving the RST, tcp_input() calls tcp_close() calls soisdisconnected() which marks the socket SS_ISDISCONNECTED and in_pcbdetach() calls sofree(), but the socket is still on the accept queue, so it is not freed. When accept() runs, it takes the socket off the accept queue and calls bsd_accept() calls soaccept() calls tcp_usr_accept() which checks SS_ISDISCONNECTED and returns ECONNABORTED. Previously, at this point the socket handle (which was copied to the return structure) was dropped and an ECONNABORTED returned. The return structure is ignored on error. The patches insert soclose() at this point. In the original free bsd code, this is called several layers down in fdrop(), which is (correctly) commented out in the port as the file allocation is done at the higher layer in eCos. Since bsd_accept() takes ownership of the socket when it takes it off the accept queue, it is appropriate to free it here. In minimumdiff.txt, this is the only change. While I was there, I also noticed the NULL checks for "name" and "anamelen" are not consistent and are made one extra time, the lower half of the function is hard to follow due to gotos, there is an extra "error=0", the now invalid socket handle is "leaked" out the new_fp structure and finally, an error returned by copyout() would also cause the socket to be lost. To correct that, I've moved and changed the NULL checks, deleted the #if 0 sections, turned the unnecessary gotos into if/else structures and moved the new_fp assignment. Handling an error returned from copyout would involve a call to soclose and moving the new_fp assignment after it; but copyout() is just memcpy and always returns true, so I've called memcpy directly and eliminated the unnecessary check. All of these changes are in fulldiff.txt We have tested the change in our products, triggering the event of interest as well as regular use of accept. None of the existing test cases seems to cover this situation, largely because it is a timing issue. Cameron Taylor Vecima Networks
Attachment:
mindiff.txt
Description: mindiff.txt
Attachment:
fulldiff.txt
Description: fulldiff.txt
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |