[lwip-devel] [bug #19162] lwip_sendto: possible to corrupt remote addr/port connection state

[Frédéric Bernon]

Follow-up Comment #21, bug #19162 (project lwip):

>re api_msg.c changes, fromport and fromaddr being used when sending. I think
their names would need changing in that case.

Yes, It would be better, but this change the netconn API on "recv", and I
wouldn't do that

>In any case, these fields would now need initialising in netbuf_new,
otherwise they may have any value.

Yes, except that now, netconn users could avoid to do the same job than in
older lwip_send (check conn state, connect, send, disconnect). In a general
way, this new patch give better performance than older (less operations), and
it's thread safe...

> But I'm uneasy about additions within the netconn API to support the BSD
API. These changes do nothing for netconn API users - they're only to fit the
BSD API requirements. Given that, I think it would therefore be better to be
like before and change the connection temporarily. I've mentioned
tcpip_callback() already a few times just this evening, but perhaps that
would be better than creating a new message type just for the BSD sockets.

The problem is that netconn & sockets have the same problem. Even if you want
to use netconn, you will got the same problem explain in first comments about
this bug. The problem is that the original design of ALL the sequential API
(netconn or sockets) wasn't do to be thread safe.

> The changed lwip_sendto seems to return lwip error codes, and not set
errno.

Yes, this is a draft, and this is not the first thing I would have to check
(the port & addr "host" or "network" order is very checked yet). But, yes,
it's necessary.

> If that's fixed, lwip_send would also need changing so errno is only set in
TCP branch.

Ok

> According to the standards, send(s,buf,len,fl) is the same as
sendto(s,buf,len,fl,NULL,0) (see "Application usage" at
http://www.opengroup.org/onlinepubs/007908799/xns/send.html ). So allowing a
NULL sockaddr in lwip_sendto would be better than having to set up a dummy
struct sockaddr in lwip_send to avoid the assert. I think lwip_send() (and
lwip_write()) can be made to just call lwip_sendto() directly.

Ok for NULL parameter for sockaddr... About the opengroup link, some others
things are missing (using NULL is only possible if you use a TCP connection,
or if the UDP connection is already "connect"ed, etc...)

Thank you for these comments



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?19162>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/