Repository navigation
Conversation
|
casaroli
left a comment
There was a problem hiding this comment.
All 20 cases of a POSIX test of connect() and poll() on datagram sockets pass on Linux; 11 of them pass with this PR on qemu-armv8a (knsh and nsh). The test is apache/nuttx-apps#3964.
Enable poll/epoll on AF_LOCAL SOCK_DGRAM sockets by creating and opening the receiver FIFO persistently at bind time, rather than temporarily at each recvfrom. This allows file_poll on lc_infile to work for DGRAM sockets, removing the -ENOSYS limitation. Changes: - local_bind: create and open receiver FIFO for DGRAM at bind time - local_netpoll: replace -ENOSYS with file_poll for DGRAM sockets Because the FIFO now outlives a single recvfrom, three further cases need handling: - lc_infile is read-only, so it never becomes writable and poll() would never report POLLOUT. A datagram send opens its own FIFO on demand and has no send buffer to wait for, so POLLOUT is reported immediately and file_poll() is used only for POLLIN. An unbound socket has no lc_infile at all, where file_poll() would have returned -ENOSYS and poll() would have reported POLLERR. - bind() opens the FIFO in the blocking mode of the socket, while psock_dgram_recvfrom() only passed MSG_DONTWAIT when it opened the FIFO itself. recv(MSG_DONTWAIT) on an empty bound socket would therefore wait forever; check FIONREAD first and return -EAGAIN, as psock_stream_recvfrom() does. - A second bind() would open a second receiver FIFO over the first, leaking its reader, and a failed bind() left lc_path and lc_type set so that a retry got EADDRINUSE from the socket itself. Reject a bind() of an already bound socket with -EINVAL and restore the unbound state on the error path. Signed-off-by: yezhonghui <yezhonghui@xiaomi.com>
3d77c1b to
d58cf73
Compare
casaroli
left a comment
There was a problem hiding this comment.
Commit eaa7b97 has the author zhanghongyu but only the Signed-off-by of yezhonghui.
Fix: add your Signed-off-by to it.
The summary says two changes and lists local_conn.c, which this PR no longer changes. It does not name drivers/pipes. It says stream sockets do not change, but a second bind() and connect(AF_UNSPEC) on a stream socket now give EINVAL.
Fix: update the summary, the impact and the file list.
| if (conn->lc_state != LOCAL_STATE_UNBOUND && | ||
| conn->lc_state != LOCAL_STATE_BOUND) | ||
| conn->lc_state != LOCAL_STATE_BOUND && | ||
| conn->lc_state != LOCAL_STATE_CONNECTED) |
There was a problem hiding this comment.
sendto() with an address on a datagram socket pair now hits an assert. A pair is LOCAL_STATE_CONNECTED, so this check lets it through, and its lc_outfile is open, so the DEBUGASSERT at line 262 fails with CONFIG_DEBUG_ASSERTIONS: "Assertion failed : at file: local/local_sendmsg.c:262" on qemu-armv8a:nsh. Master returns EISCONN.
Fix: return -EISCONN here when conn->lc_peer != NULL.
| * check the address. | ||
| */ | ||
|
|
||
| if (conn->lc_peer == NULL) |
There was a problem hiding this comment.
connect() with only sun_family (addrlen 2) returns 0, and an empty name matches any unbound socket.
Fix: return -EINVAL when addrlen <= sizeof(sa_family_t) + 1, as psock_local_bind() does.
| */ | ||
|
|
||
| return -ENOSYS; | ||
| if (stat(path, &buf) == 0) |
There was a problem hiding this comment.
stat() is the user API and sets errno.
Fix: nx_stat(path, &buf, 1), as psock_local_connect() does.
POSIX allows connect() on DGRAM sockets to set the default peer address for subsequent send() calls. The NuttX local socket implementation returned ENOSYS for DGRAM connect. Add support by storing the peer path in a new lc_daddr field and implementing send() for connected DGRAM sockets via implicit sendto() using the stored address. Also support AF_UNSPEC to dissolve the association. The default peer is recorded in a flag of its own, lc_dconnected, rather than in lc_state, so that bind() and connect() in either order do not overwrite each other, a bound socket stays under the address in-use check, and local_sendto() keeps rejecting the sockets that hold a real peer link. A socketpair() is LOCAL_STATE_CONNECTED with its outgoing FIFO already open, where the DEBUGASSERT in local_sendto() would fire. local_connect() rejected anything whose sa_family was not AF_LOCAL, so AF_UNSPEC has to be let through the entry check; SOCK_STREAM rejects it explicitly because a connection-mode socket cannot dissolve its association. The peer address is copied with the caller's addrlen rather than the full struct sockaddr_un, so that a caller supplying only sa_family does not have memory past its buffer read. local_findconn() reads sun_path[0], so an addrlen that does not even cover that byte is rejected with -EINVAL. connect() also validates the destination rather than leaving a bad name to be reported by the next send(): it returns -ENOENT when no socket has that name, or -ECONNREFUSED when the pathname exists in the file system but is not one of our sockets. That lookup uses nx_stat() rather than stat(), which is the user-space entry point and would set the caller's errno, and it is skipped for an abstract name, which has no file system entry. send() on a connected socket maps a late -ENOENT to -ECONNREFUSED, since connect() already established that the peer existed. Because connect() only records an address and does not link to a peer connection, lc_peer stays NULL. local_getpeername() checked only lc_state before reading peer->lc_type, which was a NULL dereference for a connected datagram socket; it now reports lc_daddr for a datagram socket with no peer link, chosen by socket type because a stream socket whose peer has closed also has lc_peer == NULL and must keep returning ENOTCONN. And per POSIX the peer address limits which sender a connected receiver accepts, so local_sendto() returns -EPERM when the receiver has connected to somebody else, as Linux does. local_sendto() built the FIFO name from unaddr->sun_path even for an abstract address, whose first byte is NUL, so a datagram sent to an abstract name went to the FIFO of the empty name and the receiver got nothing. Skip the leading NUL as local_findconn() does. Finally, restore the lc_sendlock around local_send_packet() on the stream path: moving local_send() below local_sendto() dropped it, which would have left two threads sending on one stream socket unserialized. Signed-off-by: yezhonghui <yezhonghui@xiaomi.com> Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
pipecommon_poll() reports POLLHUP only when the pipe is empty, has no writers and uses buffer policy 0. pipecommon_close() reported it for every policy when the last writer went away, so the two disagreed. Under policy 1 the buffered data stays valid for the next reader, so losing the last writer is not an end-of-file condition. This matters for AF_LOCAL datagram sockets, which open and close their sending FIFO for every packet: a receiver polling the FIFO saw POLLIN|POLLHUP for each datagram, and POLLHUP alone for a send that failed with EMSGSIZE, so a program that closes its socket on POLLHUP stopped after the first datagram. Apply the same policy-0 condition as pipecommon_poll(). Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
Summary
Two changes to AF_LOCAL
SOCK_DGRAMsockets, both lifting an-ENOSYS.net/local: add poll support for SOCK_DGRAM local sockets—local_pollsetup()and
local_pollteardown()returned-ENOSYSfor DGRAM because the receiverFIFO was opened only for the duration of each
recvfrom(), so there was nopersistent
lc_infileto hand tofile_poll(). Create and open the receiverFIFO in
psock_local_bind()instead, and forward poll setup/teardown tofile_poll().net/local: add connect support for DGRAM sockets— POSIX letsconnect()ona datagram socket record a default peer so that plain
send()works.local_connect()returned-ENOSYS. Store the peer in a newlc_daddrfield, allowlocal_sendto()to run inLOCAL_STATE_CONNECTED(connect only records an address, it does not open a FIFO), and have
local_send()on a DGRAM socket forward tolocal_sendto()with the storedaddress.
local_send()is moved belowlocal_sendto()so it can call it.Files:
net/local/{local.h,local_bind.c,local_conn.c,local_netpoll.c, local_sendmsg.c,local_sockif.c}.Impact
AF_LOCAL DGRAM only; stream sockets are untouched apart from
local_send()no longer sharing its body with the DGRAM case.
bind()now allocates thereceiver FIFO, so a DGRAM socket that is bound but never read holds a FIFO
for its whole lifetime, and
bind()can now fail with the errorslocal_create_halfduplex()/local_open_receiver()report.Testing
sim:udgram(CONFIG_NET_LOCAL_DGRAM=y,CONFIG_MM_KASAN=y), a test appcovering the two new capabilities plus the behaviour they must not break:
Both commits build individually, and
tools/checkpatch.sh -c -u -m -g apache/master..HEADpasses.