STMicroelectronics / STMems_Android_Sensor_HAL_IIO

DISCONTINUED (October 2022): the maintenance for this repository has been discontinued. Please refer to https://github.com/STMicroelectronics/st-mems-android-linux-sensors-hal for the up-to-date HAL repository. This repository contains ST Android sensor Hardware Abstraction Layer (HAL) for MEMS Linux IIO drivers
Apache License 2.0
35 stars 21 forks source link

Null pointer dereference using st_imu68 driver. #6

Closed kataklysmo closed 3 years ago

kataklysmo commented 3 years ago

Hi @mariotesi ,

I'm using the LSM9DS1 sensor and i added the st_imu68 IIO driver into my kernel.

https://github.com/STMicroelectronics/STMems_Linux_IIO_drivers/tree/linux-3.18-gh/drivers/iio/imu/st_imu68

Inside /sys/bus/iio/devices i can read raw devices. So the driver is working fine.

/sys/bus/iio/devices/iio:devicex/in_xxx_x_raw /sys/bus/iio/devices/iio:devicex/in_xxx_y_raw /sys/bus/iio/devices/iio:devicex/in_xxx_z_raw

Once i add the STMems_Android_Sensor_HAL_IIO the service is crashing.

01-01 00:02:12.558  2358  2358 F DEBUG   : Revision: '0'
01-01 00:02:12.558  2358  2358 F DEBUG   : ABI: 'arm'
01-01 00:02:12.558  2358  2358 F DEBUG   : pid: 2355, tid: 2355, name: android.hardwar  >>> /vendor/bin/hw/android.hardware.sensors@1.0-service <<<
01-01 00:02:12.558  2358  2358 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x1c
01-01 00:02:12.558  2358  2358 F DEBUG   : Cause: null pointer dereference
01-01 00:02:12.559  2358  2358 F DEBUG   :     r0 00000001  r1 00000000  r2 af5905c8  r3 00000000
01-01 00:02:12.559  2358  2358 F DEBUG   :     r4 af5905c8  r5 0000005a  r6 00000000  r7 00000003
01-01 00:02:12.559  2358  2358 F DEBUG   :     r8 bebad354  r9 00000004  sl 00000000  fp bebab950
01-01 00:02:12.559  2358  2358 F DEBUG   :     ip af86062c  sp bebab910  lr af8252ad  pc af371b64  cpsr 200e0030
01-01 00:02:12.619  2358  2358 F DEBUG   :
01-01 00:02:12.619  2358  2358 F DEBUG   : backtrace:
01-01 00:02:12.620  2358  2358 F DEBUG   :     #00 pc 00007b64  /vendor/lib/hw/STsensors.msm8909.so (st_hal_open_sensors(hw_module_t const*, char const*, hw_device_t**)+275)

Line 1216-SensorHal.cpp - st_hal_set_fullscale this function is using data[index].channels and in my case these channels are null.

if (data[index].sa.length > 0) {
        err = st_hal_set_fullscale(data[index].iio_sysfs_path, ST_sensors_supported[n].android_sensor_type,
                                &data[index].sa, data[index].channels, data[index].num_channels);
        if (err < 0) {
        ALOGE("\"%s\": unable to set full scale. (errno: %d)", iio_devices[i].name, err);
        goto st_hal_load_free_iio_channels;
                }
        }

The function used to initialize these channels is called in Line 1173-SensorHAL.cpp and defined in 371-utils.cpp

err = device_iio_utils::scan_channel(data[index].iio_sysfs_path,
                             &data[index].channels,
                             &data[index].num_channels);

This function looks for the scan_elements path, and in my case this path is not created by the driver.

As a workaround i just added a check before the st_hal_set_fullscale call. The driver is loading and i can see the sensors on the system (dumpsys sensorservice) . I can still read raw values but the system is not able to read any value from the sensor.

if (data[index].sa.length > 0 && data[index].channels != NULL ) {
        err = st_hal_set_fullscale(data[index].iio_sysfs_path, ST_sensors_supported[n].android_sensor_type,
                                &data[index].sa, data[index].channels, data[index].num_channels);
        if (err < 0) {
        ALOGE("\"%s\": unable to set full scale. (errno: %d)", iio_devices[i].name, err);
        goto st_hal_load_free_iio_channels;
                }
        }

Some idea of what may be happening? Thank you in advance.

Sensor Device:
Total 4 h/w sensors, 4 running:
0x00000001) active-count = 1; sampling_period(ms) = {10.0}, selected = 10.00 ms; batching_period(ms) = {0.0}, selected = 0.00 ms
0x00000002) active-count = 2; sampling_period(ms) = {66.7, 50.0}, selected = 50.00 ms; batching_period(ms) = {100.0, 0.0}, selected = 0.00 ms
0x00000003) active-count = 1; sampling_period(ms) = {5.0}, selected = 5.00 ms; batching_period(ms) = {0.0}, selected = 0.00 ms
0x00000004) active-count = 2; sampling_period(ms) = {1.0, 1.0}, selected = 1.00 ms; batching_period(ms) = {0.0, 0.0}, selected = 0.00 ms
Sensor List:
0x00000001) LSM9DS1 Magnetometer Sensor | STMicroelectronics | ver: 1 | type: android.sensor.magnetic_field(2) | perm: n/a | flags: 0x00000000
        continuous | minRate=1.00Hz | maxRate=80.00Hz | FIFO (max,reserved) = (1, 0) events | non-wakeUp |
0x00000002) LSM9DS1 Accelerometer Sensor | STMicroelectronics | ver: 1 | type: android.sensor.accelerometer(1) | perm: n/a | flags: 0x00000000
        continuous | minRate=10.00Hz | maxRate=476.19Hz | FIFO (max,reserved) = (1, 0) events | non-wakeUp |
0x00000003) LSM9DS1 Gyroscope Sensor  | STMicroelectronics | ver: 1 | type: android.sensor.gyroscope(4) | perm: n/a | flags: 0x00000000
        continuous | minRate=15.00Hz | maxRate=476.19Hz | FIFO (max,reserved) = (1, 0) events | non-wakeUp |
0x00000004) Dynamic Sensor Manager    | Google          | ver: 1 | type: android.sensor.dynamic_sensor_meta(32) | perm: n/a | flags: 0x00000007
        special-trigger | minRate=1000.00Hz | maxRate=1000.00Hz | no batching | wakeUp |
0x5f636779) Corrected Gyroscope Sensor | AOSP            | ver: 1 | type: android.sensor.gyroscope(4) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f676172) Game Rotation Vector Sensor | AOSP            | ver: 3 | type: android.sensor.game_rotation_vector(15) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f676273) Gyroscope Bias (debug)    | AOSP            | ver: 1 | type: android.sensor.accelerometer(1) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f67656f) GeoMag Rotation Vector Sensor | AOSP            | ver: 3 | type: android.sensor.geomagnetic_rotation_vector(20) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f677276) Gravity Sensor            | AOSP            | ver: 3 | type: android.sensor.gravity(9) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f6c696e) Linear Acceleration Sensor | AOSP            | ver: 3 | type: android.sensor.linear_acceleration(10) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f726f76) Rotation Vector Sensor    | AOSP            | ver: 3 | type: android.sensor.rotation_vector(11) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
0x5f797072) Orientation Sensor        | AOSP            | ver: 1 | type: android.sensor.orientation(3) | perm: n/a | flags: 0x00000000
        continuous | maxDelay=0us | maxRate=476.19Hz | no batching | non-wakeUp |
Fusion States:
9-axis fusion enabled (1 clients), gyro-rate= 200.00Hz, q=< 0, 0, 0, 0 > (0), b=< 0, 0, 0 >
game fusion(no mag) enabled (1 clients), gyro-rate= 200.00Hz, q=< 0, 0, 0, 0 > (0), b=< 0, 0, 0 >
geomag fusion (no gyro) enabled (1 clients), gyro-rate= 200.00Hz, q=< 0, 0, 0, 0 > (0), b=< 0, 0, 0 >
Recent Sensor events:
Active sensors:
LSM9DS1 Magnetometer Sensor (handle=0x00000001, connections=1)
LSM9DS1 Accelerometer Sensor (handle=0x00000002, connections=2)
LSM9DS1 Gyroscope Sensor (handle=0x00000003, connections=1)
Dynamic Sensor Manager (handle=0x00000004, connections=2)
Game Rotation Vector Sensor (handle=0x5f676172, connections=1)
GeoMag Rotation Vector Sensor (handle=0x5f67656f, connections=1)
Gravity Sensor (handle=0x5f677276, connections=1)
Linear Acceleration Sensor (handle=0x5f6c696e, connections=1)
Rotation Vector Sensor (handle=0x5f726f76, connections=1)
Orientation Sensor (handle=0x5f797072, connections=1)
Socket Buffer size = 984 events
WakeLock Status: not held
Mode : NORMAL
3 active connections
Connection Number: 0
        Operating Mode: NORMAL
         com.android.server.policy.WindowOrientationListener | WakeLockRefCount 0 | uid 1000 | cache size 0 | max cache size 0
         LSM9DS1 Accelerometer Sensor 0x00000002 | status: active | pending flush events 0
Connection Number: 1
        Operating Mode: NORMAL
         com.android.server.SensorNotificationService | WakeLockRefCount 0 | uid 1000 | cache size 0 | max cache size 0
         Dynamic Sensor Manager 0x00000004 | status: active | pending flush events 0
Connection Number: 2
        Operating Mode: NORMAL
         com.cpuid.cpu_z.MainActivity | WakeLockRefCount 0 | uid 10077 | cache size 0 | max cache size 0
         LSM9DS1 Magnetometer Sensor 0x00000001 | status: active | pending flush events 0
         LSM9DS1 Accelerometer Sensor 0x00000002 | status: First flush pending | pending flush events 0
         LSM9DS1 Gyroscope Sensor 0x00000003 | status: active | pending flush events 0
         Dynamic Sensor Manager 0x00000004 | status: active | pending flush events 0
         Game Rotation Vector Sensor 0x5f676172 | status: active | pending flush events 0
         GeoMag Rotation Vector Sensor 0x5f67656f | status: active | pending flush events 0
         Gravity Sensor 0x5f677276 | status: active | pending flush events 0
         Linear Acceleration Sensor 0x5f6c696e | status: active | pending flush events 0
         Rotation Vector Sensor 0x5f726f76 | status: active | pending flush events 0
         Orientation Sensor 0x5f797072 | status: active | pending flush events 0
0 direct connections
Previous Registrations:
mariotesi commented 3 years ago

I think the issue is in the driver because the scan_elements entries need to be created in order to describe the channel data format, otherwise the buffer is not capable to read data from sensors. I suggest you to check if CONFIG_IIO_BUFFER is enabled in iio frameworks (it should be automatically enabled by choosing the imu68 device driver checkbox in the menuconfig, anyway) and report the kernel dmesg

kataklysmo commented 3 years ago

Hi @mariotesi From the current build the flags related to the IIO using cat /proc/config.gz i attach the dmesg and also the logcat.

# CONFIG_IIO_PERIODIC_RTC_TRIGGER is not set
# CONFIG_IIO_SIMPLE_DUMMY is not set
CONFIG_IIO=y
CONFIG_IIO_BUFFER=y
CONFIG_IIO_BUFFER_CB=y
CONFIG_IIO_KFIFO_BUF=y
CONFIG_IIO_TRIGGERED_BUFFER=y
CONFIG_IIO_TRIGGER=y
CONFIG_IIO_CONSUMERS_PER_TRIGGER=2
# CONFIG_IIO_ST_ACCEL_3AXIS is not set
# CONFIG_IIO_ST_GYRO_3AXIS is not set
CONFIG_IIO_ST_IMU68=y
CONFIG_IIO_ST_IMU68_I2C=y
CONFIG_IIO_ST_IMU68_SPI=y
# CONFIG_IIO_ST_ASM330LHH is not set
# CONFIG_IIO_ST_LSM6DSO is not set
# CONFIG_IIO_ST_LSM6DSR is not set
# CONFIG_IIO_ST_ISM330DHCX is not set
# CONFIG_IIO_ST_LSM6DSO32 is not set
# CONFIG_IIO_ST_MAGN_3AXIS is not set
# CONFIG_IIO_INTERRUPT_TRIGGER is not set
# CONFIG_IIO_SYSFS_TRIGGER is not set
# CONFIG_IIO_ST_PRESS is not set

The scan_elements folder is not present. About SELinux is on permissive mode so i have multiple avc messages pending to be fixed. Here a example from the logcat file.

01-01 00:11:36.439   414   414 I         : Loaded library from /vendor/lib/hw/STsensors.msm8909.so
01-01 00:11:36.479   421   421 I ServiceManagement: Removing namespace from process name vendor.qti.hardware.qteeconnector@1.0-service to qteeconnector@1.
01-01 00:11:36.487   414   414 I Sensors : Found calibration library:libcalmodule_common.so
01-01 00:11:36.509   414   414 I android.hardwar: type=1400 audit(0.0:6): avc: denied { write } for name="/" dev="mmcblk0p38" ino=2 scontext=u:r:hal_sensors_default:s0 tcontext=u:object_r:system_data_file:s0 tclass=dir permissive=1
01-01 00:11:36.509   414   414 I android.hardwar: type=1400 audit(0.0:7): avc: denied { add_name } for name="STSensorHAL" scontext=u:r:hal_sensors_default:s0 tcontext=u:object_r:system_data_file:s0 tclass=dir permissive=1
01-01 00:11:36.509   414   414 I android.hardwar: type=1400 audit(0.0:8): avc: denied { create } for name="STSensorHAL" scontext=u:r:hal_sensors_default:s0 tcontext=u:object_r:system_data_file:s0 tclass=dir permissive=1
01-01 00:11:36.509   414   414 I android.hardwar: type=1400 audit(0.0:9): avc: denied { read } for name="devices" dev="sysfs" ino=16143 scontext=u:r:hal_sensors_default:s0 tcontext=u:object_r:sysfs:s0 tclass=dir permissive=1
01-01 00:11:36.509   414   414 I android.hardwar: type=1400 audit(0.0:10): avc: denied { open } for path="/sys/bus/iio/devices" dev="sysfs" ino=16143 scontext=u:r:hal_sensors_default:s0 tcontext=u:object_r:sysfs:s0 tclass=dir permissive=1

Thank you.

kataklysmo commented 3 years ago

Hi again @mariotesi,

I added a print just to show that the channels are NULL once STMems_Android_Sensor_HAL_IIO loads.

01-01 02:36:49.155   416   416 D SensorHAL: 3 IIO devices available into /sys/bus/iio/devices/ folder.
01-01 02:36:49.155   416   416 D SensorHAL: "lsm9ds1_accel": IIO device found and supported. Wake-up sensor: no
01-01 02:36:49.158   416   416 D SensorHAL: SensorHal.cpp:1216- data[index].channels is NULL!

The st_imu68 driver doesn't create the scan_elements folder and the function scan_channel from STMems_Android_Sensor_HAL_IIO (utils.cpp) checks if the directory exists.

int device_iio_utils::scan_channel(const char *device_dir,
                   struct device_iio_info_channel **data,
                   int *counter)

    memset(dfilename, 0, DEVICE_IIO_MAX_FILENAME_LEN + 1);
    snprintf(dname, DEVICE_IIO_MAX_FILENAME_LEN, "%s/scan_elements",
         device_dir);

    ret = sysfs_opendir(dname, &dp);

and that generates the crash once STMems_Android_Sensor_HAL_IIO loads as i pointed before.

mariotesi commented 3 years ago

Yes I see, but the issue is in the imu driver, it must create the entries scan_elements otherwise is not possible ti use the buffer I think there are some issue on buffer allocation in your kernel 3,18 driver so I suggest to add some kernel printk in the imu68 driver during the probe and in the init buffer, devm_request_threaded_irq, just to check where the driver fails (maybe the irq is not configured in the device tree ?)

kataklysmo commented 3 years ago

And just to try to help, I found a comment from a year ago where you indicate that the st_imu68 driver does not create the directory scan_elements it uses the entry path or at least that's what I understand from the comment, I'll will add some printk on the imu68 driver following your suggestion. image

About the IRQ it's configured on the device tree.

lsm9ds1_ag@6b {
        compatible = "st,lsm9ds1";
        reg = <0x6B>;
        interrupt-parent = <&msm_gpio>;
        interrupts = <65>;
};
lsm9ds1_m@1e{
        compatible = "st,lsm9ds1_magn";
        reg = <0x1E>;
        interrupt-parent = <&msm_gpio>;
        interrupts = <94>;
};
kataklysmo commented 3 years ago

Hi @mariotesi ,

Just adding some printk as you suggested, the IRQ was not correctly defined. For anyone with the same problem i just added the interupt-names field. Now the interrupt is registered and the scan_elements created.

    lsm9ds1_ag@6b {
        compatible = "st,lsm9ds1";
        reg = <0x6B>;
        interrupt-parent = <&msm_gpio>;
        interrupts = <65>;
        interrupt-names = "ag_irq";
};

Thanks for the support.

mariotesi commented 3 years ago

Thanks you too !