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

[Jonathan Larmour]

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

>> Fortunately users should not be accessing these fields directly - that's
> what netbuf_fromaddr() and netbuf_fromport() are for :-). 
> 
> Yes, but to my point of view, these functions are not a great idea,
because
> you can directly access to fields (and doing that is faster and reduce
> footprint).

Maybe then they could be made macros, to preserve the API abstraction - but
that's a separate issue. Functions like this are intentional to hide
implementation details.

But certainly right now, people are meant to use the functions. It's ok to
change the netbuf field names.

>> here are thread-safety issues with the netconn API in some places, but
not
> this one - netconn_connect and _send are straightforward...
> 
> I'm not agree with you on this point: if your comment about a TCP
connection
> is true about segments order (it have to be process by application and/or
> protocol), I think it's wrong for UDP connections. First, an application
> could want to send the same datagram to multiple destinations (if
multicast
> isn't supported, or if WAN is used, by example). But with current
sequential
> api (netconn or sockets) performance is poor (need to check, connect,
> disconnect, exchange lot of messages between api_lib/api_msg...), but raw
api
> allows it (see udp_sendto & raw_sendto, they already do the job at low
level).

You are unlikely to need to check actually - the app will usually already
know where the connection was set to point to. You also don't need to
disconnect - just like lwip_sendto, you can call netconn_connect again.

So it's just a two step process: netconn_connect and netconn_send. I think
it's not common at all to send the same datagram to multiple destinations. I
think it's even rarer to also need to do it from multiple overlapping
threads. Personally it doesn't seem like something common enough to be
bending over backwards to add support in the APIs for. And for the rare users
who do need this *and* need to do it from multiple overlapping threads - they
can just do their own protection - the race condition is fairly obvious
because it's a two step process (unlike inside lwip_sendto which presents
only a single API function).

> Next, if two threads want to send data using the same socket or netconn,
with
> both we can got the problem described by Rob in the initial description.

Only if it is the same socket to multiple destinations.

Anyway, I realise I probably sound like I have stronger feelings on this than
I actually do. I did say before I was only "uneasy" about it. If you are
really sure about it, I'm okay with that - I just wanted you to hear a
different point of view.

I think if you want to make this capability available to netconn users, then
there should probably be a netconn_sendto() function that wraps netconn_send,
but takes and fills in the address/port in the netbuf (which should still
either be renamed from fromaddr/fromport, or made a union with something like
destaddr/destport).

> We have already talk about that, but I think that two sequential api, with
> one based on the other one, is not a good idea: I think it will be better
to
> keep the BSD one, and to add some extra functions for zero-copy. So, most
of
> the code will be the same, footprint will be reduced (for BSD users, of
> course :) ), less code will have to be maintain, keep source code
> compatibility with most of open source applications...). But this last
> comment is another subject (and a bigger task)... (if you want to continue
to
> talk about that, I propose to continue on the forum).

As you probably know I disagree :-). I don't think you can sensibly add zero
copy to a BSD socket API. At the very least, once you try, you haven't got a
BSD socket API. There might be an argument for implementing BSD directly over
the raw API, but if it's doing largely the same things as the netconn API,
that wouldn't give much benefits and makes things larger if anyone uses both
APIs at the same time. Anyway, another time for that discussion.

Jifl


    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/