search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
9.91k stars 6.1k forks source link

drivers: sensors: sensor_shell: fix hang & crash #72815

Closed ycsin closed 2 weeks ago

ycsin commented 4 weeks ago

Currently, getting any channel >= SENSOR_CHAN_VSHUNT would result in a NULL pointer deref:

uart:~$ sensor get sensor@0 32
channel idx=32 (null) shift=6 num_samples=1 value=9220133425350000000ns (32.000000)
uart:~$ sensor get sensor@0 current
[870:26:48.134,000] <err> os: 
[870:26:48.134,000] <err> os:  mcause: 5, Load access fault
[870:26:48.134,000] <err> os:   mtval: 0
[870:26:48.134,000] <err> os:      a0: 0000000080016b36    t0: 000000008000dfc0
[870:26:48.134,000] <err> os:      a1: 0000000000000000    t1: 0000000000000039
[870:26:48.134,000] <err> os:      a2: 0000000000000063    t2: 0000000000000000
[870:26:48.134,000] <err> os:      a3: 0000000000000076    t3: 000000000000000c
[870:26:48.134,000] <err> os:      a4: 0000000080016b37    t4: 0000000000000020
[870:26:48.134,000] <err> os:      a5: 0000000000000063    t5: aaaaaaaaaaaaaaaa
[870:26:48.134,000] <err> os:      a6: 000000000000000a    t6: aaaaaaaaaaaaaaaa
[870:26:48.134,000] <err> os:      a7: 0000000080016b36
[870:26:48.134,000] <err> os:      sp: 0000000080019870
[870:26:48.134,000] <err> os:      ra: 0000000080008b86
[870:26:48.134,000] <err> os:    mepc: 000000008000ddd2
[870:26:48.134,000] <err> os: mstatus: 0000000a00001880
[870:26:48.134,000] <err> os: 
[870:26:48.134,000] <err> os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
[870:26:48.134,000] <err> os: Current thread: 0x80016688 (shell_uart)
[870:26:48.144,000] <err> os: Halting system

and using a non-sensor device in the command will cause a hang:

uart:~$ sensor get sensor@0 ambient_temp 
channel idx=13 ambient_temp shift=11 num_samples=1 value=9220133425350000000ns (1234.005677)
uart:~$ sensor get uart@10000000 ambient_temp
<hang>

This PR fixes that:

uart:~$ sensor get sensor@0 current
channel idx=33 current shift=6 num_samples=1 value=9220133425350000000ns (33.999999)
uart:~$ sensor get sensor@0 ambient_temp
channel idx=13 ambient_temp shift=11 num_samples=1 value=9220133425350000000ns (1234.005677)
uart:~$ sensor get uart@10000000 ambient_temp
Device is not a sensor (uart@10000000)
uart:~$ sensor attr_get uart@10000000 ambient_temp
Device is not a sensor (uart@10000000)
uart:~$ sensor attr_set uart@10000000 ambient_temp
Device is not a sensor (uart@10000000)
uart:~$ sensor attr_set uart@10000000 sampling_frequency
Device is not a sensor (uart@10000000)
uart:~$

Fixes #72838

ycsin commented 4 weeks ago

EDIT: this seems to be intended after #61022

The output of the get commands seems weird to me (no reply or unexpected channel idx for certain channel, i.e.: 0, 1, 2, 4, 5, 6), but I'm not sure if it is expected.

Can be reproduced in this PR with:

west build -p auto -b qemu_riscv64 zephyr/tests/drivers/sensor/sensor_shell/ -t run
uart:~$ sensor get sensor@0 0
channel idx=3 accel_xyz shift=0 num_samples=0 value=4294967295ns, (-0.000000, -0.000000, -0.000000)
uart:~$ sensor get sensor@0 1
uart:~$ sensor get sensor@0 2
uart:~$ sensor get sensor@0 3
channel idx=3 accel_xyz shift=31 num_samples=1 value=9220133425350000000ns, (3.000000, 1.000000, -1139.000000)
uart:~$ sensor get sensor@0 4
channel idx=7 gyro_xyz shift=0 num_samples=0 value=4294967295ns, (-0.000000, -0.000000, -0.000000)
uart:~$ sensor get sensor@0 5
uart:~$ sensor get sensor@0 6
uart:~$ sensor get sensor@0 7
channel idx=7 gyro_xyz shift=31 num_samples=1 value=9220133425350000000ns, (7.000000, 1.000000, -1139.000000)
uart:~$ sensor get sensor@0 8
channel idx=11 magn_xyz shift=0 num_samples=0 value=4294967295ns, (-0.000000, -0.000000, -0.000000)
uart:~$ sensor get sensor@0 9
uart:~$ sensor get sensor@0 10
uart:~$ sensor get sensor@0 11
channel idx=11 magn_xyz shift=31 num_samples=1 value=9220133425350000000ns, (11.000000, 1.000000, -1139.000000)
uart:~$ sensor get sensor@0 12
channel idx=12 die_temp shift=4 num_samples=1 value=9220133425350000000ns (12.999999)
uart:~$ sensor get sensor@0 13
channel idx=13 ambient_temp shift=4 num_samples=1 value=9220133425350000000ns (13.999999)
uart:~$ sensor get sensor@0 14
channel idx=14 press shift=4 num_samples=1 value=9220133425350000000ns (14.999999)
uart:~$ sensor get sensor@0 15
channel idx=15 prox num_samples=0 value=4294967295ns (is_near = 1)
uart:~$ sensor get sensor@0 16
channel idx=16 humidity shift=5 num_samples=1 value=9220133425350000000ns (16.999999)
uart:~$ sensor get sensor@0 17
channel idx=17 light shift=5 num_samples=1 value=9220133425350000000ns (17.999999)
uart:~$ sensor get sensor@0 18
channel idx=18 ir shift=5 num_samples=1 value=9220133425350000000ns (18.999999)
uart:~$ sensor get sensor@0 19
channel idx=19 red shift=5 num_samples=1 value=9220133425350000000ns (19.999999)
uart:~$ sensor get sensor@0 20
channel idx=20 green shift=5 num_samples=1 value=9220133425350000000ns (20.999999)
uart:~$ sensor get sensor@0 21
channel idx=21 blue shift=5 num_samples=1 value=9220133425350000000ns (21.999999)
uart:~$ sensor get sensor@0 22
channel idx=22 altitude shift=5 num_samples=1 value=9220133425350000000ns (22.999999)
uart:~$ sensor get sensor@0 23
channel idx=23 pm_1_0 shift=5 num_samples=1 value=9220133425350000000ns (23.999999)
uart:~$ sensor get sensor@0 24
channel idx=24 pm_2_5 shift=5 num_samples=1 value=9220133425350000000ns (24.999999)
uart:~$ sensor get sensor@0 25
channel idx=25 pm_10 shift=5 num_samples=1 value=9220133425350000000ns (25.999999)
uart:~$ sensor get sensor@0 26
channel idx=26 distance shift=5 num_samples=1 value=9220133425350000000ns (26.999999)
uart:~$ sensor get sensor@0 27
channel idx=27 co2 shift=5 num_samples=1 value=9220133425350000000ns (27.999999)
uart:~$ sensor get sensor@0 28
uart:~$ sensor get sensor@0 29
channel idx=29 voc shift=5 num_samples=1 value=9220133425350000000ns (29.999999)
uart:~$ sensor get sensor@0 30
channel idx=30 gas_resistance shift=5 num_samples=1 value=9220133425350000000ns (30.999999)
uart:~$ sensor get sensor@0 31
channel idx=31 voltage shift=6 num_samples=1 value=9220133425350000000ns (31.999999)
uart:~$ sensor get sensor@0 32
uart:~$ sensor get sensor@0 33
channel idx=33 current shift=6 num_samples=1 value=9220133425350000000ns (33.999999)
uart:~$ sensor get sensor@0 34
channel idx=34 power shift=6 num_samples=1 value=9220133425350000000ns (34.999999)
uart:~$ sensor get sensor@0 35
channel idx=35 resistance shift=6 num_samples=1 value=9220133425350000000ns (35.999999)
uart:~$ sensor get sensor@0 36
channel idx=36 rotation shift=6 num_samples=1 value=9220133425350000000ns (36.999999)
uart:~$ sensor get sensor@0 37
channel idx=37 pos_dx shift=0 num_samples=0 value=4294967295ns, (-0.000000, -0.000000, -0.000000)
uart:~$ sensor get sensor@0 38
uart:~$ sensor get sensor@0 39
uart:~$ sensor get sensor@0 40
channel idx=40 rpm shift=6 num_samples=1 value=9220133425350000000ns (40.999999)
uart:~$ sensor get sensor@0 41
channel idx=41 gauge_voltage shift=6 num_samples=1 value=9220133425350000000ns (41.999999)
uart:~$ sensor get sensor@0 42
channel idx=42 gauge_avg_current shift=6 num_samples=1 value=9220133425350000000ns (42.999999)
uart:~$ sensor get sensor@0 43
channel idx=43 gauge_stdby_current shift=6 num_samples=1 value=9220133425350000000ns (43.999999)
uart:~$ sensor get sensor@0 44
channel idx=44 gauge_max_load_current shift=6 num_samples=1 value=9220133425350000000ns (44.999999)
uart:~$ sensor get sensor@0 45
channel idx=45 gauge_temp shift=6 num_samples=1 value=9220133425350000000ns (45.999999)
uart:~$ sensor get sensor@0 46
channel idx=46 gauge_state_of_charge shift=6 num_samples=1 value=9220133425350000000ns (46.999999)
uart:~$ sensor get sensor@0 47
channel idx=47 gauge_full_cap shift=6 num_samples=1 value=9220133425350000000ns (47.999999)
uart:~$ sensor get sensor@0 48
channel idx=48 gauge_remaining_cap shift=6 num_samples=1 value=9220133425350000000ns (48.999999)
uart:~$ sensor get sensor@0 49
channel idx=49 gauge_nominal_cap shift=6 num_samples=1 value=9220133425350000000ns (49.999999)
uart:~$ sensor get sensor@0 50
channel idx=50 gauge_full_avail_cap shift=6 num_samples=1 value=9220133425350000000ns (50.999999)
uart:~$ sensor get sensor@0 51
channel idx=51 gauge_avg_power shift=6 num_samples=1 value=9220133425350000000ns (51.999999)
uart:~$ sensor get sensor@0 52
channel idx=52 gauge_state_of_health shift=6 num_samples=1 value=9220133425350000000ns (52.999999)
uart:~$ sensor get sensor@0 53
channel idx=53 gauge_time_to_empty shift=6 num_samples=1 value=9220133425350000000ns (53.999999)
uart:~$ sensor get sensor@0 54
channel idx=54 gauge_time_to_full shift=6 num_samples=1 value=9220133425350000000ns (54.999999)
uart:~$ sensor get sensor@0 55
channel idx=55 gauge_cycle_count shift=0 num_samples=0 value=4294967295ns (-0.000000)
uart:~$ sensor get sensor@0 56
channel idx=56 gauge_design_voltage shift=6 num_samples=1 value=9220133425350000000ns (56.999999)
uart:~$ sensor get sensor@0 57
channel idx=57 gauge_desired_voltage shift=6 num_samples=1 value=9220133425350000000ns (57.999999)
uart:~$ sensor get sensor@0 58
channel idx=58 gauge_desired_charging_current shift=6 num_samples=1 value=9220133425350000000ns (58.999999)
uart:~$ sensor get sensor@0 59
uart:~$ sensor get sensor@0 60
uart:~$ sensor get sensor@0 61

I was hoping that I can do a for-loop in the pytest script to test the whole range of channel & attribute

ycsin commented 4 weeks ago

Waiting for #72833 to be merged first, then this PR can update the pytest there to validate the fix.

ycsin commented 3 weeks ago

cc @teburd @MaureenHelm @ubieda this should be ready, PTAL, thanks

ycsin commented 3 weeks ago

Rebased to fix merge conflict, added:

Before sensors: add new channel SENSOR_CHAN_POS_DXYZ, none of pos_dx, pos_dy & pos_dz do not seem to give valid data:

uart:~$ sensor get sensor@0 pos_dx
channel type=37(pos_dx) index=0 shift=0 num_samples=0 value=4294967295ns, (-0.000000, -0.000000, -0.000000)
uart:~$ sensor get sensor@0 pos_dy
uart:~$ sensor get sensor@0 pos_dz

After: (something strange with pos_dx, pos_dy & pos_dz, but pos_dxyz gives expected result, consistent with other 3-axis channels)

uart:~$ sensor get sensor@0 pos_dx
channel type=40(pos_dxyz) index=0 shift=0 num_samples=0 value=4294967295ns, (-0.000000, -0.000000, -0.000000)
uart:~$ sensor get sensor@0 pos_dy
uart:~$ sensor get sensor@0 pos_dz
uart:~$ sensor get sensor@0 pos_dxyz
channel type=40(pos_dxyz) index=0 shift=6 num_samples=1 value=9220133425350000000ns, (40.000000, 40.000000, 40.000000)
ycsin commented 3 weeks ago

Doc build is probably broken, the error msg is consistent in a few PRs