This allows MMKernelDevice::cmp to compare two kernel devices
with different object types, which means that subclasses can
continue to only handle comparisons with their own type. The
order may not be stable across builds.
E.g. it may be one more layer up:
looking at device '/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.4/1-8.4:1.1/ttyUSB1/tty/ttyUSB1':
KERNEL=="ttyUSB1"
SUBSYSTEM=="tty"
DRIVER==""
looking at parent device '/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.4/1-8.4:1.1/ttyUSB1':
KERNELS=="ttyUSB1"
SUBSYSTEMS=="usb-serial"
DRIVERS=="option1"
ATTRS{port_number}=="0"
looking at parent device '/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.4/1-8.4:1.1':
KERNELS=="1-8.4:1.1"
SUBSYSTEMS=="usb"
DRIVERS=="option"
ATTRS{bNumEndpoints}=="02"
ATTRS{supports_autosuspend}=="1"
ATTRS{bInterfaceNumber}=="01"
ATTRS{bAlternateSetting}==" 0"
ATTRS{bInterfaceSubClass}=="06"
ATTRS{bInterfaceProtocol}=="13"
ATTRS{interface}=="Huawei Mobile Connect - Application"
ATTRS{bInterfaceClass}=="ff"
ATTRS{authorized}=="1"
kerneldevice/mm-kernel-device-udev.c: In function ‘mm_kernel_device_udev_new_from_properties’:
kerneldevice/mm-kernel-device-udev.c:770:70: error: declaration of ‘properties’ shadows a global declaration [-Werror=shadow]
770 | mm_kernel_device_udev_new_from_properties (MMKernelEventProperties *properties,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
kerneldevice/mm-kernel-device-udev.c:38:20: note: shadowed declaration is here
38 | static GParamSpec *properties[PROP_LAST];
| ^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c: In function ‘mm_kernel_device_generic_new_with_rules’:
kerneldevice/mm-kernel-device-generic.c:954:68: error: declaration of ‘properties’ shadows a global declaration [-Werror=shadow]
954 | mm_kernel_device_generic_new_with_rules (MMKernelEventProperties *properties,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c:46:20: note: shadowed declaration is here
46 | static GParamSpec *properties[PROP_LAST];
| ^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c: In function ‘mm_kernel_device_generic_new’:
kerneldevice/mm-kernel-device-generic.c:971:57: error: declaration of ‘properties’ shadows a global declaration [-Werror=shadow]
971 | mm_kernel_device_generic_new (MMKernelEventProperties *properties,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
kerneldevice/mm-kernel-device-generic.c:46:20: note: shadowed declaration is here
46 | static GParamSpec *properties[PROP_LAST];
| ^~~~~~~~~~
This prevents errors due to nasty typos in the strings.
We define all symbols in a single header file that is NOT considered
part of the API, as there is no need for MM clients to know about
these tags code-wise. These tags are only meaningful when associated
to devices in udev.
Information of each tag is included in the general API documentation.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/issues/88
This new object allows configuring the filter rules applied to the
device ports. By default, for now, it implements the same rules as the
MMKernelDevice is_candidate() method, which is obsoleted.
This patch implicitly enables in the generic device backend the
manual-only greylist (ID_MM_DEVICE_MANUAL_SCAN_ONLY tag) and the
platform TTY whitelist (ID_MM_PLATFORM_DRIVER_PROBE), which were not
being applied.
A USB device may report its manufacturer through the Manufacturer String
Descriptor. That information can sometimes be useful, e.g. MBIM doesn't
define a standard command to query the manufacturer. This patch adds a
method to MMKernelDevice to retrieve the manufacturer information from
the underlying physical device, which can later be used by
MMBroadbandModemMbim to report the manufacturer of a MBIM modem.
==5075== 198 bytes in 18 blocks are definitely lost in loss record 4,486 of 4,761
==5075== at 0x4C2CE5F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5075== by 0x66E3028: g_malloc (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x66CAD42: g_path_get_basename (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x20BB7B: preload_interface_sysfs_path (mm-kernel-device-generic.c:184)
==5075== by 0x20C885: preload_contents (mm-kernel-device-generic.c:383)
==5075== by 0x20E2E1: check_preload (mm-kernel-device-generic.c:830)
==5075== by 0x20EA6F: set_property (mm-kernel-device-generic.c:953)
==5075== by 0x6455359: ??? (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6456DBC: g_object_new_valist (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6105E95: g_initable_new_valist (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x6105F58: g_initable_new (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x20E848: mm_kernel_device_generic_new_with_rules (mm-kernel-device-generic.c:901)
==5075== 333 bytes in 37 blocks are definitely lost in loss record 4,612 of 4,761
==5075== at 0x4C2CE5F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5075== by 0x66E3028: g_malloc (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x66CAD42: g_path_get_basename (in /usr/lib/libglib-2.0.so.0.5200.3)
==5075== by 0x20BE20: preload_driver (mm-kernel-device-generic.c:242)
==5075== by 0x20C8E5: preload_contents (mm-kernel-device-generic.c:391)
==5075== by 0x20E2E1: check_preload (mm-kernel-device-generic.c:830)
==5075== by 0x20EA6F: set_property (mm-kernel-device-generic.c:953)
==5075== by 0x6455359: ??? (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6456DBC: g_object_new_valist (in /usr/lib/libgobject-2.0.so.0.5200.3)
==5075== by 0x6105E95: g_initable_new_valist (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x6105F58: g_initable_new (in /usr/lib/libgio-2.0.so.0.5200.3)
==5075== by 0x20E848: mm_kernel_device_generic_new_with_rules (mm-kernel-device-generic.c:901)
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.
g_free and g_object_unref are in form of `void (*)(gpointer)`, which
matches the GDestroyNotify signature. An explicit GDestroyNotify cast on
g_free and g_object_unref is thus not needed.
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.
This is so that "make check" doesn't depend on having the rule files
installed in the default udev rules directory, which currently would
break the build:
TEST: test-service-generic... (pid=2601)
/MM/Service/Generic/enable-disable: Activating service name='org.freedesktop.ModemManager1'
Successfully activated service 'org.freedesktop.ModemManager1'
** (/home/aleksander/Development/foss/ModemManager/plugins/.libs/lt-test-service-generic:2601):
ERROR **: Error setting test profile: GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Failed: Failed to find primary AT port
FAIL
GTester: last random seed: R02S3897abaae9df36f8d2eeb679406ab675
make[3]: *** [Makefile:3804: test-nonrecursive] Terminated
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>
We need at least ID_USB_INTERFACE_NUM, as some plugins (Huawei, Telit, Option)
require this property when gathering port type hints.
We also add support for other properties (ID_VENDOR_ID, ID_MODEL_ID...) as we
already have those values preloaded.
Instead of relying on the udev daemon and GUDev to manage the devices reported
by the kernel, we can now run ModemManager relying solely on the kernel events
reported via the new ReportKernelEvent() API. Therefore, the '--no-auto-scan'
option is implicit for the ModemManager daemon when udev is disabled in the
build.
Additionally, a new custom implementation of the kernel device object is
provided, which uses sysfs to load the properties and attributes required in
each kernel device, instead of using a GUdevDevice.
The udev rule files are kept in place, and a simple custom parser is provided
which preloads all rules in memory once and then applies them to the different
kernel objects reported via ReportKernelEvent(), e.g. to set port type hints.
A simple unit test setup is prepared to validate the udev rules during the
`check' Makefile target.
The mm_base_modem_grab_port() now receives a MMKernelDevice directly from the
plugin, which is then stored in the MMPort corresponding to the port.
This means that we have direct access to e.g. all properties set by udev rules
everywhere, and we don't need additional GUdevClient objects (e.g. like the one
used in the Huawei plugin to detect NDISDUP support during runtime).
For virtual ports (e.g. generated during unit tests), we have a new 'generic'
kernel device object which just provides the values from the kernel device
properties given during its creation.
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.
Eric Caruso
Aleksander Morgado
Baruch Siach
Ben Chan