[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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.
