Add a new property to ubus_subscriber called new_obj_cb. It gets called
everytime a new object appears on the bus. If the callback returns true,
libubus will automatically subscribe to the notifications.
Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Defers and continues a client's lookup command to avoid unnecessary
buffering under load.
Signed-off-by: Daniel Danzberger <daniel@dd-wrt.com>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
When a built-in object is invoked with UBUS_ATTR_DATA absent, recv_msg
will be called with NULL as the msg argument and ubusd_forward_event
and ubusd_alloc_event_pattern need to handle this. Otherwise, a
truncated invoke of "send" or "register" on UBUS_SYSTEM_OBJECT_EVENT
that is missing UBUS_ATTR_DATA will cause ubusd to crash with SIGSEGV.
Signed-off-by: Erik Karlsson <erik.karlsson@genexis.eu>
'ubus list [<path>]' passes the path to ubusd,
this commit fix the lua bindings to do the same
Signed-off-by: Etienne Champetier <champetier.etienne@gmail.com>
When the ubus "call" command fails, ubus prints "Command failed: ...".
This is fine when the call command is executed interactively by the
user. However, when the call command is executed non-interactively,
these log messages leave the user completely clueless:
netifd: wan6 (10841): Command failed: Unknown error
procd: /etc/rc.d/S80umdns: Failed to parse message data
These messages contain absolutely no info that explains where they come
from; it's not even clear they are coming from ubus. This makes tracking
down what's causing them virtually impossible.
Improve this situation by printing the full ubus call to stderr when
stderr is not a TTY.
Example of improved error message:
netifd: wan (2836): Command failed: ubus call network.interface notify_proto { "action": 0, "link-up": false, "keep": false, "interface": "wan" } (Permission denied)
While one might argue not to include the JSON message in the log,
excluding it will still not help to reproduce the failed command.
As we only print the full command when stderr is not a TTY, seeing this
error means we're doing a bad ubus call somewhere, which is a bug and
should be fixed. Printing the full command makes more sense than
printing half the command and still requiring us to figure out the exact
command that failed.
Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Petr Štetiar <ynezz@true.cz>
Acked-by: Jo-Philipp Wich <jo@mein.io>
Use the newly introduced UBUS_STATUS_PARSE_ERROR status message in
ubus_cli_call() and ubus_cli_send().
Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Petr Štetiar <ynezz@true.cz>
Acked-by: Jo-Philipp Wich <jo@mein.io>
Introduce the following new status messages in libubus:
UBUS_STATUS_NO_MEMORY
UBUS_STATUS_PARSE_ERROR
UBUS_STATUS_SYSTEM_ERROR
Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Petr Štetiar <ynezz@true.cz>
Acked-by: Jo-Philipp Wich <jo@mein.io>
Avoid running into an issue where issuing a request from an obj message
can lead to recursive message processing and list corruption
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Process pending messages before attempting to read new ones. After completing
the poll, process any remaining pending messages.
A previous message processing call which issued a request from within
its handler may have left behind more object messages to process.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
This makes it easier to notice ubusd (and so often a system) failing to
start properly. Some users reported procd failing to initialize fully
without a clear error in case of faulty /etc/passwd.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: John Crispin <john@phrozen.org>
Use the correct parameters for list_add_tail when enqueueing a message
in the tx_queue. Previously, list_add_tail(list, entry) was used instead
of list_add_tail(entry, list). Due to this, the list would only contain
the latest entry, and previously inserted entries were left dangling.
Reset the tx_queue offset after a message from the tx_queue has been sent
completely.
Signed-off-by: Alexander Van Parys <alexander.vanparys_ext@softathome.com>
A bad client can send a message whose blob_attr len is less than 4,
and ubus_msg_new happily points ->data off the end of the allocated
buffer, leading to invalid reads, writes, and eventually a crash if
ubus monitor is running:
==17683== Invalid write of size 4
==17683== at 0x10A915: client_cb (ubusd_main.c:143)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683== Address 0x4a63200 is 0 bytes after a block of size 32 alloc'd
==17683== at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683== by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683== by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 4
==17683== at 0x10A645: blob_len (blob.h:102)
==17683== by 0x10A93D: blob_raw_len (blob.h:111)
==17683== by 0x10A93D: client_cb (ubusd_main.c:149)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683== Address 0x4a63200 is 0 bytes after a block of size 32 alloc'd
==17683== at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683== by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683== by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 4
==17683== at 0x10ACE8: blob_len (blob.h:102)
==17683== by 0x10B7E1: blob_raw_len (blob.h:111)
==17683== by 0x10B7E1: ubusd_proto_receive_message (ubusd_proto.c:457)
==17683== by 0x10A9A7: client_cb (ubusd_main.c:169)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683== Address 0x4a63200 is 0 bytes after a block of size 32 alloc'd
==17683== at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683== by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683== by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 4
==17683== at 0x10D39B: blob_len (blob.h:102)
==17683== by 0x10D53E: ubusd_monitor_message (ubusd_monitor.c:91)
==17683== by 0x10A99C: client_cb (ubusd_main.c:168)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683== Address 0x4a6b3e0 is 0 bytes after a block of size 32 alloc'd
==17683== at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683== by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683== by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 1
==17683== at 0x4848286: blob_put (blob.c:167)
==17683== by 0x10D555: ubusd_monitor_message (ubusd_monitor.c:91)
==17683== by 0x10A99C: client_cb (ubusd_main.c:168)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683== Address 0x4a6b3e4 is 4 bytes after a block of size 32 alloc'd
==17683== at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683== by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683== by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683==
==17683== Process terminating with default action of signal 11 (SIGSEGV)
==17683== Bad permissions for mapped region at address 0x4E43000
==17683== at 0x4848286: blob_put (blob.c:167)
==17683== by 0x10D555: ubusd_monitor_message (ubusd_monitor.c:91)
==17683== by 0x10A99C: client_cb (ubusd_main.c:168)
==17683== by 0x48495E3: uloop_run_events (uloop.c:198)
==17683== by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683== by 0x10A503: uloop_run (uloop.h:111)
==17683== by 0x10A503: main (ubusd_main.c:284)
The following Python program minimally reproduces the issue:
import socket
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
sock.connect('/tmp/usock')
sock.recv(12)
sock.send(b'\x00\x04\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00')
Signed-off-by: Julian Squires <julian@cipht.net>
No new message can be enqueued if this brings the total queue length of
that client over UBUS_CLIENT_MAX_TXQ_LEN.
Set UBUS_CLIENT_MAX_TXQ_LEN to UBUS_MAX_MSGLEN, i.e. 1MB. This limit
should be plenty for any practical use case.
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
ubusd maintains a per-client tx_queue containing references to message
buffers that have not been sent yet (due to the socket blocking). This
is a fixed-size, 64-element queue.
When more than 64 elements are queued, subsequent elements are simply
dropped. Thus, a client that is waiting for those messages will block
indefinitely. In particular, this happens when more than +- 250 objects
are registered on the bus and either "ubus list" or "ubus wait_for" is
called. The responses to these requests consist of a message buffer per
object. Since in practice, ubusd will not yield between the sends of
these message buffers, the client has no time to process them and
eventually the output socket blocks. After 64 more objects, the rest is
dropped, including the final message that indicates termination. Thus,
the client waits indefinitely for the termination message.
To solve this, turn the tx_queue into a variable-sized linked list
instead of a fixed-size queue.
To maintain the linked list, an additional structure ubus_msg_buf_list
is created. It is not possible to add the linked list to ubus_msg_buf,
because that is shared between clients.
Note that this infinite tx_queue opens the door to a DoS attack. You can
open a client and a server connection, then send messages from the
client to the server without ever reading anything on the server side.
This will eventually lead to an out-of-memory. However, such a DoS
already existed anyway, it just requires opening multiple server
connections and filling up the fixed-size queue on each one. To protect
against such DoS attacks, we'd need to:
- keep a global maximum queue size that applies to all rx and tx queues
together;
- stop reading from any connection when the maximum is reached;
- close any connection when it hasn't become writeable after some
timeout.
Fixes: https://bugs.openwrt.org/index.php?do=details&task_id=1525
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Add a new `ABIVERSION` define which allows to control the SOVERSION used
for the built shared library. This is needed for downstream packaging to
properly track breaking ABI changes when updating to newer versions of
the library.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
When ubus is running as root, /var/run/ubus most likely hasn't been
created as well (as that's the homedir of user ubus, and if a user
ubus was found, then ubus would run being that user).
Blindly attempt to create the directory (which won't do any harm if
it does exist and/or ubus is not running as root) to still be able to
start ubus in case of user ubus not existing.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Add support for wildcard in methods to permit access to all methods
defined by the object. This can be usefull for process that run as
non-root user and needs to access ubus method.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Support for "subscribe" command was added years ago in the commit
453b87f631 ("cli: add support for subscribing to objects"). Document
its usage.
Cc: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
If the Lua number exceeds the maximum value representable by an
unsigned 32bit integer, store it in an unsigned 64bit integer
field instead.
Signed-off-by: Alin Nastac <alin.nastac@gmail.com>
[align code style, reword commit message]
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
In commit 5d7ca8309d ("ubusd/libubus-io: fix variable sized struct
position warning") the position of cmsghdr struct has been changed in
order to fix clang-9 compiler warning, but it has introduced regression
in at least `logread` which hanged indefinitely.
So this patch reworks the socket descriptor passing in a way recommended
in the `cmsg(3)` manual page.
Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-December/020840.html
Fixes: 5d7ca8309d ("ubusd/libubus-io: fix variable sized struct position warning")
Reported-by: Hannu Nyman <hannu.nyman@welho.com>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
In commit 08f17c87a0 ("add fuzzer and cram based unit tests") some
fuzz/unit tests were added so enable them on CI as well.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
blob_parse expects blobs from trusted inputs, but it can be supplied
with possibly malicious blobs from untrusted inputs as well, which might
lead to undefined behaviour and/or crash of ubus daemon. In order to
prevent such conditions, switch to blob_parse_untrusted which should
hopefully handle such untrusted inputs appropriately.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
scan-build from clang-9 has reported following:
libubox/list.h:141:2: warning: Use of memory after it is freed
_list_add(_new, head, head->next);
Signed-off-by: Petr Štetiar <ynezz@true.cz>
This dereference could possibly happen if the calloc call fails as the
return value is unchecked. While at it refactor the code little bit to
make it easier to follow, use safe list iterator and provide return
value for ubusd_monitor_connect.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
scan-build from clang-9 has reported following:
libubox/list.h:83:22: warning: Use of memory after it is freed
entry->next->prev = entry->prev;
^~~~~~~~~~~
ubusd_event.c:42:3: warning: Use of memory after it is freed
ubusd_delete_event_source(ev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Which might be a false positives, but in order to make the code pass the
static analyzer checks, rewrite the while loops on lists with the safe
list iterator.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
scan-build from clang-9 has reported following:
ubus.c:837:16: warning: Access to field 'rnotify' results in a dereference of a null pointer (loaded from variable 'sub')
sub->rnotify = luaL_ref(L, -2);
Which is false positive as the lua_error() does a long jump and
therefore never returns and this long jump probably confuses the static
analyzer. So this patch workarounds this false positive by helping
static analyzer by using common Lua idiom which is to return
lua_error()'s return value.
Ref: https://www.lua.org/manual/5.1/manual.html#lua_error
Addresses-Coverity-ID: 1412355 ("Dereference after null check")
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following error reported by clang-9 analyzer:
examples/server.c:244:2: warning: Value stored to 'argc' is never read
argc -= optind;
^ ~~~~~~
examples/server.c:245:2: warning: Value stored to 'argv' is never read
argv += optind;
^ ~~~~~~
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Uses currently proof-of-concept openwrt-ci[1] in order to:
* improve the quality of the codebase in various areas
* decrease code review time and help merging contributions faster
* get automagic feedback loop on various platforms and tools
- out of tree build with OpenWrt SDK on following targets:
* ath79-generic
* imx6-generic
* malta-be
* mvebu-cortexa53
- out of tree native build on x86/64 with GCC (versions 7, 8, 9) and Clang 10
- out of tree native x86/64 static code analysis with cppcheck and
scan-build from Clang 9
1. https://gitlab.com/ynezz/openwrt-ci/
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following error reported by clang-9 analyzer:
libubus.c:286:19: error: incompatible pointer types assigning to 'struct blob_attr *' from 'char *' [-Werror,-Wincompatible-pointer-types]
ctx->msgbuf.data = (char *) calloc(UBUS_MSG_CHUNK_SIZE, sizeof(char));
Result of 'calloc' is converted to a pointer of type 'struct blob_attr',
which is incompatible with sizeof operand type 'char'.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
clang-9 on x86/64 has reported following warnings/errors:
libubus-acl.c:123:2: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
libubus-io.c:108:18: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
libubus-io.c:395:56: error: comparison of integers of different signs: 'ssize_t' (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
libubus-req.c:441:4: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:119:18: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd_acl.c:152:5: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:348:3: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:352:3: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:357:3: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:362:3: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:367:3: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
ubusd_acl.c:447:16: error: comparison of integers of different signs: 'int' and '__size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
ubusd_acl.c:502:18: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd.c:123:13: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd.c:170:15: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd.c:262:43: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd.c:287:30: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd_event.c:170:18: error: comparison of integers of different signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
ubusd_obj.c:71:2: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following clang-9 compiler warnings:
ubusd.c:99:18: error: field 'h' with variable sized type 'struct cmsghdr' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
struct cmsghdr h;
^
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following clang-9 compiler warning:
ubusd.c:36:19: error: comparison of integers of different signs: 'uint32_t' (aka 'unsigned int') and 'int' [-Werror,-Wsign-compare]
if (ub->refcount == ~0) {
~~~~~~~~~~~~ ^ ~~
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Let's enforce additional automatic checks enforced by the compiler in
order to catch possible errors during compilation.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
If the header is read but not the remainder of the message, the stream
will be out of sync and parsing of future messages won't work
Signed-off-by: Felix Fietkau <nbd@nbd.name>
