Date: Tue, 14 Jun 2022 08:19:18 +0300 From: Timo Teras <timo.teras@....fi> To: Rich Felker <dalias@...c.org> Cc: musl@...ts.openwall.com, Waldek Kozaczuk <jwkozaczuk@...il.com> Subject: Re: netlink.c: missing handling of EAGAIN and EWOULDBLOCK On Mon, 13 Jun 2022 13:08:49 -0400 Rich Felker <dalias@...c.org> wrote: > On Mon, Jun 13, 2022 at 11:41:57AM -0400, Waldek Kozaczuk wrote: > > Very recently we implemented minimal rnetlink support on OSv side > > which allowed us to finally switch to the netlink-based > > implementation of getifaddrs() and if_nameindex(). > > > > However, I noticed that the function __netlink_enumerate() in > > https://github.com/ifduyue/musl/blob/master/src/network/netlink.c > > uses MSG_DONTWAIT flag when calling recv() which may fail with > > EAGAIN or EWOULDBLOCK and there is no error/retry handling for > > that. I actually saw both functions fail occasionally on OSv. In linux the kernel generates the responses during recv() syscall, which guarantees that there is no EAGAIN or EWOULDBLOCK unless the operation has completed and there really is no longer any response data left. IIRC, I added the MSG_DONTWAIT just to be protective against bad implementation to not block indefinitely if NLMSG_DONE/NLMSG_ERROR is not received or parsed properly. This could happen e.g. if the assumption about buffer size no longer is true. But seems 8kB is now documented as upper buffer size universally. > > One way to fix is to add missing error handling. But another simpler > > solution is to stop using MSG_DONTWAIT altogether and force recv() > > to block. In other words, the line: > > > > r = recv(fd, u.buf, sizeof(u.buf), MSG_DONTWAIT); > > > > should change to: > > > > r = recv(fd, u.buf, sizeof(u.buf), 0); This should work equally well on Linux if the kernel response is parsed correctly and contains a terminating message. > > For time being we are applying a header trick on OSv side to > > re-define MSG_DONTWAIT as 0 when compiling those specific musl > > sources. > > Thanks! I'll try to track this down. One concern is that I'm not sure > how MSG_DONTWAIT is supposed to interact with "short reads" -- is it > needed (for netlink) to prevent blocking when some data has been read > but there is still buffer space for more? > > On a related issue, I'm pretty sure the netlink API doesn't allow for > partial reads with some data remaining buffered on the kernel side, > but we should probably verify that too. The netlink socket is datagram-oriented. The kernel has special measures on how the datagrams are generated. Currently NLM_F_DUMP requests generate maximum size of NLMSG_GOODSIZE (capped to 8kB by code and docs) to make sure no super large buffers are needed. And as mentioned, the current implementation generates additional response datagrams during the syscall if possible. So the socket buffer size does not really effect things here. (The netlink socket buffer size affects only sockets listening for events where the messages are really buffered.) Another way to detect the wrong-sized buffer is to use MSG_TRUNC and make sure the returned value is not larger than buffer size. If that happens, then the buffer size is not enough. But given the current man page, and stability of the Linux implementation. I think we can assume the 8kB buffer size and current code is good. I would not have objection on the suggested change. However, if we do support alternative implementations, it might be good to document the assumptions made. I wonder if the buffer size check is needed in case the alternate implementation used something different? Timo
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.