get_device_ids() in mm-kernel-device-udev.c accepts a NULL 'vendor' or
'product' argument, but the current implementation could result in a
potential NULL dereferences of the 'vendor' argument. Given that
get_device_ids() is a local helper and its only caller provides a
non-NULL 'vendor' and 'product' argument, this patch removes the NULL
checks (i.e. get_device_ids() expects non-NULL 'vendor' and 'product').
This patch also rearranges the code such that the 'vendor' argument is
updated only when the function returns TRUE, just like how the 'product'
argument is handled.
There are 2 main types of udev properties: device-specific and
port-specific.
The port-specific properties are set independently per port (e.g. port
type hints set per interface number for a given vid:pid).
The device-specific properties apply to all ports in the device. Some
of these properties are currently expected in the physical device
(e.g. ID_MM_PLATFORM_DRIVER_PROBE) while some others are expected in
each port (e.g. the plugin udev tag filters).
This patch tries to simplify the logic and just assume that the device
specific tags may be given in either the physical device or the port
device, by providing separate APIs to retrieve port-specific or
device-specific (global) properties. If the same tag is given in both
the device and the port, the one in the device takes preference.
For the generic backend, these new APIs are really useless, as all
device-specific and port-specific properties are always stored in the
port object themselves (there is no 'tree' of devices in the generic
backend, no 'physdev' device).
For the udev backend, though, there really is a difference, as the
tags may be set in port or device.
https://bugs.freedesktop.org/show_bug.cgi?id=100156
find_device_by_physdev_uid() expects a non-NULL UID of the physical
device. However, mm_kernel_device_get_physdev_uid() could potentially
return NULL on MMKernelDeviceUdev if find_physical_gudevdevice() in
mm-kernel-device-udev.c fails to find the physical device. When a NULL
physical device UID is passed to find_device_by_physdev_uid() and used
in g_hash_table_lookup(), it leads to a crash in g_str_hash(), which is
a bit obscure.
This patch updates kernel_device_get_physdev_uid() in
mm-kernel-device-udev.c to fall back to use the device sysfs if the
physical device sysfs isn't available, which is similar to how
kernel_device_get_physdev_uid() mm-kernel-device-generic.c is
implemented.
This patch simplifies the handling of platform/pci/pnp/sdio device in
find_physical_gudevdevice(). When the code finds the first parent device
under the subsystem 'platform', 'pci', 'pnp', or 'sdio', it stops
traversing up the device tree, so there is no need to keep track of
whether a platform / pci / pnp / sdio device has previously been visited
via those is_platform/is_pci/is_pnp/is_sdio Boolean variables.
A few crashes have been observed in the field with the following signature:
Thread 0 CRASHED [SIGSEGV @ 0x00000000 ] MAGIC SIGNATURE THREAD
0xf53ff5e8 (libglib-2.0.so.0.3600.4 -ghash.c:1732 ) g_str_hash
0xf53fe8c7 (libglib-2.0.so.0.3600.4 -ghash.c:365 ) g_hash_table_lookup
0xb953c3bd (ModemManager -mm-base-manager.c:130 ) device_removed
0xb953cc9f (ModemManager -mm-base-manager.c:408 ) handle_uevent
0xf54c535f (libgobject-2.0.so.0.3600.4 -gclosure.c:777 ) g_closure_invoke
0xf54d6fc7 (libgobject-2.0.so.0.3600.4 -gsignal.c:3584 ) signal_emit_unlocked_R
0xf54d7baf (libgobject-2.0.so.0.3600.4 -gsignal.c:3328 ) g_signal_emit_valist
0xf54d7fa5 (libgobject-2.0.so.0.3600.4 -gsignal.c:3384 ) g_signal_emit
0xf5623819 (libgudev-1.0.so.0.2.0 -gudevclient.c:104 ) monitor_event
0xf540cc51 (libglib-2.0.so.0.3600.4 -gmain.c:3054 ) g_main_context_dispatch
0xf540cfa5 (libglib-2.0.so.0.3600.4 -gmain.c:3701 ) g_main_context_iterate
0xf540d2c5 (libglib-2.0.so.0.3600.4 -gmain.c:3895 ) g_main_loop_run
0xb9539dad (ModemManager -main.c:180 ) main
0xf52d687d (libc-2.23.so -libc-start.c:289 ) __libc_start_main
0xb9539bf3 (ModemManager + 0x0001cbf3 ) _start
0xb95b0fbf (ModemManager -elf-init.c:87 ) __libc_csu_init
0xf56dbe43 (ld-2.23.so + 0x0000be43 ) _dl_sort_fini
0xb9539bbf (ModemManager + 0x0001cbbf ) _init
The crash happens when mm-kernel-device-udev.c:find_physical_gudevdevice()
fails to find the physical device, which eventually leads to a NULL
`physdev_uid' being passed to g_hash_table_lookup() in
mm-base-manager.cc:find_device_by_physdev_uid().
There isn't much information about the actual device that triggered the
udev event, but it's suspected to be a tty or net device on a SDIO bus
and thus associated with a physical device on the 'sdio' subsystem. This
patch updates find_physical_gudevdevice() in mm-kernel-device-udev.c to
handle this particular scenario.
The original g_udev_device_get_property_as_int() uses strtol() without
an explicit base (i.e. 0) so that the base is autodetected from the
string whenever possible (e.g. if prefixes with '0x' it is treated as a
hexadecimal string).
But, for ID_USB_INTERFACE_NUM, we explicitly require reading the number
as an hex string, even if we don't have any '0x' prefix.
Reported-by: Matthew Stanger <stangerm2@gmail.com>
This commit enables a new core ModemManager daemon option, so that automatic
detection of available modems is totally disabled: '--no-auto-scan'. Note that
this option also replaces the previously used '--test-no-auto-scan' option,
which was only used during tests.
Along with the new ModemManager option, a new ReportKernelEvent() method in
the API is defined, which allows notifying the daemon of which interfaces it
should be accessing, as well as the main details of each interface. The only
mandatory parameters in the new method are 'action' (add/remove), 'name' (the
name of the interface) and 'subsystem' (the subsystem of the interface).
The mmcli tool has support for using the new api method via several new options:
* The '--report-kernel-event' option allows specifying device ports one by
one, and is a direct mapping of the ReportKernelEvent() method:
$ sudo mmcli --report-kernel-event="action=add,name=wwan0,subsystem=net"
$ sudo mmcli --report-kernel-event="action=add,name=cdc-wdm0,subsystem=usbmisc"
* The '--report-kernel-event-auto-scan' option uses udev monitoring to notify
events automatically to the daemon. This allows to operate in a way
equivalent to the default daemon operation (with implicit auto-scan).
Worth noting that the ReportKernelEvent() method is only usable when
'--no-auto-scan' is explicitly used in the daemon. An error will be reported if
the method is tried while standard udev monitoring is enabled (implicit if
auto scan isn't explicitly disabled in the daemon).
If mmcli is going to be used only to report 'real time' events, an optional
'--initial-kernel-events=[PATH]' may be given in the ModemManager call to
automatically process a set of port kernel events one by one on boot. The file
may e.g. contain:
action=add,name=wwan0,subsystem=net
action=add,name=cdc-wdm0,subsystem=usbmisc
Instead of relying constantly on GUdevDevice objects reported by GUdev, we now
use a new generic object (MMKernelDevice) for which we provide an initial GUdev
based backend.