The MSG_PEEK API of recvmsg() should be avoid because it requires an additional
syscall. But worse is to choose a too small buffer size and failing to receive
the message.
A user who is aware of the issue can avoid MSG_PEEK by either
nl_socket_disable_msg_peek()/nl_socket_enable_msg_peek() or by setting
a buffer size via nl_socket_set_msg_buf_size().
By default however we now use MSG_PEEK. This is more important since commit
90c6ebec9b where the link dump request can be rather large.
Signed-off-by: Thomas Haller <thaller@redhat.com>
glibc provides two versions of strerror_r(), which
can be chosen between using feature test macros
_GNU_SOURCE and _POSIX_C_SOURCE. libnl is built using
the former, hence we get the glibc special version,
and all code so far has been written for this.
Other C libraries like musl on the other hand only try
to be posix compliant, and only ever provide the posix
version of strerror_r(), which has a different signature.
Uses in libnl hence generally cause printf() of an *int*
with a *string format* specifier for that reason.
Additionally, strerror_r() has been deprecated:
http://austingroupbugs.net/view.php?id=655
Switch to using strerror_l() (via our wrapper just
introduced).
Signed-off-by: André Draszik <adraszik@tycoint.com>
Reviewed-by: Stephane Ayotte <sayotte@tycoint.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
Fixes alignment, lines with just whitespace and mixed tab/space
indentation. Searched, reviewed and optionally fixed in vim using
regex ^\t* [\t ]*\([^*us ]\|$\)\|\t\+$
http://lists.infradead.org/pipermail/libnl/2016-August/002183.html
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Signed-off-by: Thomas Haller <thaller@redhat.com>
Currently, we try to handle MSG_CTRUNC, but if msg_controllen is zero, we make
double free for the same address.
realloc(0, 0) returns non-zero address
realloc(addr, 0) returns zero and free(addr) has already been called
Then we call free(addr) again and get an error like this:
*** Error in `./task_diag_all': double free or corruption (fasttop): 0x0000000000f9c160 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x77e9d)[0x7f360ed96e9d]
/lib64/libc.so.6(+0x7f53c)[0x7f360ed9e53c]
/lib64/libc.so.6(cfree+0x4c)[0x7f360eda2e9c]
/lib64/libnl-3.so.200(nl_recv+0x221)[0x7f360f2f6361]
/lib64/libnl-3.so.200(nl_recvmsgs_report+0x555)[0x7f360f2f6a95]
/lib64/libnl-3.so.200(nl_recvmsgs+0x9)[0x7f360f2f6d89]
./task_diag_all[0x400f8d]
/lib64/libc.so.6(__libc_start_main+0xf0)[0x7f360ed3f790]
./task_diag_all[0x401169]
http://lists.infradead.org/pipermail/libnl/2015-September/001965.html
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Thomas Haller <thaller@redhat.com>
When generating a port fails a few times (because they are already in used
outside of libnl's knowledge), we would back off generating a local
port and instead let kernel decide.
There was however a bug in nl_connect() that caused an assertion:
BUG at file position socket.c:147:_nl_socket_used_ports_release_all
app: socket.c:147: _nl_socket_used_ports_release_all: Assertion `0' failed.
Fixes: 96e1e5bdc2
libnl allows the user to explicitly set the local port before connecting
the socket. A more convenient way is to leave the local port unspecified
and let libnl generate a port id.
As it is, generate_local_port() would try at most 1024 ports, that
means if a user tries to connect more sockets, the automatism will
fail.
Kernel also supports choosing the local port itself (via netlink_autobind()).
So, this could be fixed by always leaving the port unspecified and let
kernel decide on the port. For that we could entirely drop generate_local_port().
There are however problems with that:
- it is unclear why generate_local_port() was even introduced in the
first place instead of always relying kernel. This code already
appeared in libnl-1, so maybe there was a good reason for it or
it is necessary on some kernel versions.
- The deprecated libnl-1 library also uses a form of generate_local_port().
Its first guess would always be getpid(), but the problem is that
it would not retry on EADDRINUSE. Currently libnl-3 generates ports in
a different sequence and will not generate a conflicting port (until it
already exhausted 1016 other ports).
Hence, currently if your application uses libnl1 and libnl3
together, the automatism might just work without conflicts
(commit 1f734a8f89).
Accidently, kernel/netlink_autobind() also first tries the process
id as port. That means, if we change libnl-3 to leave the decision
to kernel, and
- the application connects sockets both via libnl-1 and libnl-3
- and the libnl-3 socket happens to connect first
then the libnl-1 socket would fail to connect without retrying
another port.
- Removing generate_local_port() entirely changes behavior in the
following case:
sk = nl_socket_alloc();
/* accessing local port before connecting the socket used to
* freeze the local port to the generated value. */
port = nl_socket_get_local_port(sk);
nl_connect(sk, NETLINK_...);
Maybe the issues are minor and it would simplify the code just to get
rid of the cruft. But instead fix the issue without changing behavior.
Just keep trying with generate_local_port() first, before fallback to
kernel.
Reported-by: Julien Courtat <julien.courtat@6wind.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
http://lists.infradead.org/pipermail/libnl/2015-June/001889.html
When running out of local ports, _nl_socket_generate_local_port_no_release()
would leave the socket with port UINT32_MAX. That means if nl_connect()
fails due to out-of-ports, it would leave the port id assigned to an
invalid port and the socket instance was not re-usable until the user
called nl_socket_set_local_port(). Fix that by resetting the local port
to zero.
Thereby, also change generate_local_port() to return zero when
running out of ports. zero is a more natural value for ~no port found~.
It also matches the port that _nl_socket_generate_local_port_no_release()
uses when failing to generate a port.
Also ensure that zero cannot be returned as valid port by generate_local_port().
Arguably, that would only be possible if (getpid() & 0x3FFFFF)
returns zero. Just be extra cautious.
Signed-off-by: Thomas Haller <thaller@redhat.com>
CMSG_NXTHDR requires <linux/socket.h>. This fix a build error with the musl
C library:
```
undefined reference to `__cmsg_nxthdr'
```
https://github.com/thom311/libnl/pull/83
If recvmsg indicates that the message read was truncated libnl retries
to read the complete message after increasing the message buffer. This
only works if the message flags MSG_PEEK | MSG_TRUNC are set. If
NL_MSG_PEEK is not enabled on the nl_sock structure, flags are left
empty and the rest of the truncated message is discarded, hence a
subsequent recvmsg returns the next message (in case of a multipart
message, the NLMSG_DONE) is read and returned.
This patch aborts message processing if the message was truncated and
the NL_MSG_PEEK flags was not activated for the nl_sock structure.
http://lists.infradead.org/pipermail/libnl/2015-June/001888.html
[thaller@redhat.com: add NL_CAPABILITY_NL_RECV_FAIL_TRUNK_NO_PEEK]
Signed-off-by: Thomas Egerer <hakke_007@gmx.de>
Signed-off-by: Thomas Haller <thaller@redhat.com>
This is based on the patch by sagil@infinidat.com, but heavily modified.
Add a function nl_socket_set_fd(), I renamed it from nl_connect_fd().
Now nl_connect() and nl_socket_set_fd() are implemented independently as
they share little code. But they have similar functionality:
to initialize a libnl socket and set it's file descriptor.
A user who wants libnl to setup the socket can continue to use nl_connect().
A user with special requirements should setup the socket entirely. That includes
calling socket() (with or without SOCK_CLOEXEC), bind(), setting buffer size.
For the same reason I dropped nl_create_fd(). It didn't do much more then
calling socket() -- which the user can do directly.
https://github.com/thom311/libnl/pull/68
Signed-off-by: Thomas Haller <thaller@redhat.com>
- Added option to create socket (fd) without bind.
It is now possible to forward the socket fd to another child process...
...later use nl_connect_fd() to connect to socket from the child process.
- Added option to disable CLOEXEC even if defined (in socket.h)
'nl_socket_enable_cloexec' & 'nl_socket_disable_cloexec'
No change to current default behavior.
Signed-off-by: Sagi Lowenhardt <sagil@infinidat.com>
s_local.nl_pid is used to track the generated port unless NL_OWN_PORT is set.
Ensure that getsockname() doesn't overwrite the value and possibly reset
the local port manually to release the generated port.
Signed-off-by: Thomas Haller <thaller@redhat.com>
The flag was not actually used.
NL_SOCK_BUFSIZE_SET was only set by nl_socket_set_buffer_size().
Note that you can only call nl_socket_set_buffer_size() on a socket that
is already connected via nl_connect().
On first call, nl_connect() would always see NL_SOCK_BUFSIZE_SET unset, and
call nl_socket_set_buffer_size().
Since the flag was never unset, when trying to connect a socket a second
time, we would not set the buffer size again. Which was a bug.
Signed-off-by: Thomas Haller <thaller@redhat.com>
nl_syserr2nlerr() reduces a lot of platform errors to NLE_FAILURE --
"Unspecific failure" which makes it somehow hard to track down the real reason
behind a failure.
Logging them with level of 4 makes it a little less painful.
https://github.com/thom311/libnl/pull/65
Signed-off-by: Thomas Haller <thaller@redhat.com>
nl_pickup() converts error codes from netlink into
nl error codes using nl_syserr2nlerr(). The latter function
mangles different error codes to the same nl error code.
Add a new function, that returns both the nl error code
and the original error code.
Acked-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Thomas Haller <thaller@redhat.com>
It can easily happen that the generated local netlink port is alrady in
use. In that case bind will fail with ADDRINUSE.
Users of libnl3 could workaround this, by managing the local ports
themselves, but sometimes these users are libraries too and they also
don't know which ports might be used by other components.
This patch changes that nl_socket_alloc() no longer initilizes the local
port id immediately. Instead it will be initialized when the user calls
nl_socket_get_local_port() the first time and thereby shows interest in
the value.
If bind() fails with ADDRINUSE, check if the user ever cared about the
local port, i.e. whether the local port is still unset. If it is still
unset, assume that libnl should choose a suitable port and retry until
an unused port can be found.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Assuming that the kernel does not send more than a page is no longer valid,
and enabling MSG_PEEK'ing by default to figure out the exact message buffer
requirements can have a negative influence on the performance of existing
applications. Bumping the default receive buffer space to 4 pages seems
a sane default.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Previously 0 was returned which gave the caller no chance of detecting
when a non-blocking socket would block. If a caller intends to never
see an error message it should utilize poll()/select() to only read
when the socket has pending data or information.
Reported-by: Holger Eitzenberger <holger@eitzenberger.org>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Up to now only calls to nl_send_auto() could be overwritten with
nl_cb_overwrite_send(). This patch extends the capability to
nl_send()
Signed-off-by: Thomas Graf <tgraf@redhat.com>
Make nl_sendto() return NLE_INVAL if provided buffer is NULL
and make it return NLE_BAD_SOCK if the socket is not connected.
Add note in docs about lack of NL_CB_MSG_OUT invokation
1. all cleanup actions (like free()) now located at the end of function
2. in case of error or EOF, *buf and *creds (if given) set to NULL
This protect from invalid code at user's side, like:
char *buf;
x = nl_recv(..., &buf, ...);
if (x<=0)
goto cleanup;
cleanup:
free(buf);
3. all intermediate buffers are stored into local variables, and user's
variables only touches at the end.
Based on clang diagnostics:
1. lib/nl.c: recvmsgs(): nla filling with zeros commented.
2. lib/route/classid.c: & lib/route/pktloc.c:
remove zero-filling of struct stat
3. lib/route/qdisc/htb.c: Fix htb_qdisc_msg_fill(): fix zero-filling
4. ematch/container.c: container_parse:
commented why only 4 bytes are copied
len marked as unused to eliminate compiler warning
This may happen when passing connected socket to nl_cache_mngr_alloc().
Now, nl_connect() will return error trying to connect already connected socket.
Also, dont call close(-1) if socket() fails.
1. Fix some places where unsigned value compared < 0
2. Fix obsolete %Z specifier to more portable %z
3. Some erroneous types substitution
4. nl_msec2str() - 64-bit msec is now properly used,
Only safe changes. I mean int <--> uint32_t and signed/unsigned fixes.
Some functinos require size_t argument instead of int, but changes of
signatures of that functions is terrible thing.
Also, I do not pretend for a full list of fixes.
Just to shut up clang -Wall -Wextra
One more thing. ifindex. I don't change that because changes will
be too big for simple fix.
- changes the modules hierarchy to better represent the set of libaries
- list the header file that needs to be included
- remove examples/doc from api ref that is included in the guide
- add references to the guide
- fix doxygen api linking for version 1.8.0
- readd doxygen mainpage to config file
- fix a couple of doxygen doc bugs
Apparently the change to have nl_recvmsgs() return the number of
parsed messages broke nl_wait_for_ack() among other applications.
This patch reverts to the old behaviour and provides a new function
nl_recvmsgs_report() which provides the additional information for
use by the cache manager and possibly other applications.
Reported-by: Scott Bonar <sbonar@cradlepoint.com>
Signed-off-by: Thomas Graf <tgraf@redhat.com>
