buglwIP - A Lightweight TCP/IP stack - Bugs: bug #43739, Accept not reporting errors about...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #43739: Accept not reporting errors about aborted connections

Submitted by:  Joel Cunningham <jcunningham>
Submitted on:  Tue 02 Dec 2014 05:18:26 PM UTC  
 
Category: sockets/netconnSeverity: 2 - Minor
Item Group: Change RequestStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.4.1

Tue 05 Apr 2016 07:43:52 PM UTC, comment #5:

Fixed: netconn_accept() returns ERR_ABRT (sockets: ECONNABORTED) for aborted connections, ERR_CLSD (sockets: EINVAL) if the listening netconn is closed, which better seems to follow the standard.
The only downside is that ERR_ABRT is not stored in conn->last_err (since that would block further acceptes: ERR_ABRT is fatal) -> task #13922 Remove fatal error handling

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 23 Mar 2016 08:59:54 PM UTC, comment #4:

I've started to work on this by passing 'tcp_alloc()' failures on listen-input to the accept-callback (this is the first time the accept callback is called with err != ERR_OK).

ATTENTION: accept callbacks now have to handle NULL pointers for pcb, but any well-written code would already have checked this :-)

Next step is to fix netconn/socket API to handle these errors when reading 'acceptmbox' and returning an appropriate error (ERR_ABRT would be the correct but this is still marked as FATAL...)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 25 Feb 2015 09:09:53 PM UTC, comment #3:

> After looking up the opengroup specification for accept(), that lead me to consider this a bug.


Well, that spec also says it may fail if 'There was insufficient memory available to complete the operation', so this might be optional as well...

Anyway, I'd gladly accept a patch for this :-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 03 Dec 2014 03:46:10 PM UTC, comment #2:

I would agree out-of-pcbs should be included in this failure (also out-of-sockets). The accept() call in sockets.c will return ENFILE if out of sockets.

From the application point of view, if a pending connection can't be handed off due to failure to allocate connection state (either socket, netconn, or pcb) it would make sense to be consistent with error handling.

When I encountered this, I had no idea why the connection was being aborted on the server. It took considerable debugging in api_lib.c, api_msg.c, and tcp_in.c to pinpoint the failure. There are no LWIP_ASSERTs for this failure.

After looking up the opengroup specification for accept(), that lead me to consider this a bug. I would consider it low priority since it just makes life easier for application developers.

I'm not sure how the backlog should play into this since a full backlog usually doesn't result in a RST, but ignoring the SYN or ignoring resulting ACK from listener's SYN+ACK. The Linux backlog behavior is an interesting data point: http://veithen.github.io/2014/01/01/how-tcp-backlog-works-in-linux.html

Joel Cunningham <jcunningham>
Project Member
Tue 02 Dec 2014 08:45:29 PM UTC, comment #1:

Well... what's the big difference between "out-of-netconns" (internally: connection is established and then aborted) and "out-of-pcbs" (internally: SYN is RST directly)?

Whend fixing the "out-of-netconns", the "out-of-pcbs" should be fixed, too.

In any case, when returning an error here, it should be enforced to use a listen-backlog. Or maybe only return an error when a backlog is used?

After all, I'm not really sure whether returning an error is the correct thing to do here... What do you need this error for, anyway?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 02 Dec 2014 05:18:26 PM UTC, original submission:

I've discovered what appears to be a bug in accept's handling of aborted connections due to exhaustion of netconns.

As per the Open group spec, I would expect an error to be returned when a new connection fails the accept process:

http://pubs.opengroup.org/onlinepubs/009695399/functions/accept.html

I'm not sure if the correct return value should be ECONNABORTED or ENFILE (or even ENOMEM)

In my particular case, the listener is establishing a new connection, but the final step in tcp_process(), where the PCB is put into ESTABLISHED state, TCP_EVENT_ACCEPT() is called and that returns ERR_ABRT because allocation of a netconn failed (accept_function() returns ERR_MEM). After that, the new connection is aborted, but no error is reported on the listening netconn/socket.

It looks like errors from within TCP are reported via the tcp_err callback, but this would be hard to do in tcp_process() because there is no linkage between the new connected PCB and the listener PCB. This makes me think that maybe the error should be reported in accept_function() where the allocation failure happens. Thoughts?

Joel Cunningham <jcunningham>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by jcunningham (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 05 Apr 2016 07:43:52 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Wed 23 Mar 2016 08:59:54 PM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon
    Wed 25 Feb 2015 09:09:53 PM UTCgoldsimonSeverity3 - Normal=>2 - Minor
      Item GroupFaulty Behaviour=>Change Request

    Back to the top


    Powered by Savane 3.1-cleanup1