RPi Gateaway: Dropping root privilege

[alexvanbelle]

Hi,

I'm not able to quickly understand the code base.

I guess the problem is with transport (call to _transportSM.currentState->Update())
Any idea of what kind of call might create the segfault when user is not root (since everything works fine with root on this system)

My setup is:

• Hardware: Raspberry pi 1B
• OS: Raspbian
• Mysensors: latest release (2.3.0)

User is already member of groups GPIO and SPI.

Any hint would be appreciated :)

Alexandre

[rozpruwacz]

I run mysgw with gdb and for me it segfaults when trying to access gpio:

(gdb) run
Starting program: /usr/local/bin/mysgw 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
Sep 29 13:04:51 INFO  Starting gateway...
Sep 29 13:04:51 INFO  Protocol version - 2.3.0
Sep 29 13:04:51 DEBUG MCO:BGN:INIT GW,CP=RPNGL---,VER=2.3.0
Sep 29 13:04:51 DEBUG TSF:LRT:OK
Sep 29 13:04:51 DEBUG TSM:INIT
Sep 29 13:04:51 DEBUG TSF:WUR:MS=0

Program received signal SIGSEGV, Segmentation fault.
bcm2835_peri_read (paddr=0x7) at drivers/BCM/bcm2835.c:123
123			ret = *paddr;
(gdb) bt
#0  bcm2835_peri_read (paddr=0x7) at drivers/BCM/bcm2835.c:123
#1  bcm2835_st_read () at drivers/BCM/bcm2835.c:1126
#2  bcm2835_delayMicroseconds (micros=1) at drivers/BCM/bcm2835.c:449
#3  0x0002ce5c in BCMClass::digitalWrite (this=<optimized out>, gpio=<optimized out>, value=<optimized out>) at drivers/BCM/BCM.cpp:62
#4  0x0002d7ac in RPiClass::digitalWrite (this=<optimized out>, physPin=physPin@entry=24 '\030', value=value@entry=1 '\001') at drivers/BCM/RPi.cpp:56
#5  0x0001c22c in hwDigitalWrite (value=1 '\001', pin=24 '\030') at ./hal/architecture/Linux/MyHwLinuxGeneric.cpp:158
#6  RFM69_initialise (frequencyHz=868000000) at ./drivers/RFM69/new/RFM69_new.cpp:206
#7  0x00020da4 in transportInit () at ./hal/transport/RFM69/MyTransportRFM69.cpp:26
#8  stInitUpdate () at ./core/MyTransport.cpp:100
#9  0x000258a4 in transportUpdateSM () at ./core/MyTransport.cpp:384
#10 transportProcess () at ./core/MyTransport.cpp:464
#11 transportWaitUntilReady (waitingMS=0) at ./core/MyTransport.cpp:453
#12 _begin () at ./core/MySensorsCore.cpp:155
#13 0x00012f2c in main (argc=1, argv=0x7efff4c4) at ./hal/architecture/Linux/MyMainLinuxGeneric.cpp:447

On RPI platform the gpio kernel driver is not used, instead gpio is accessed with mmap :(. When I forced the build to use gpio kernel driver all works without problem when running as non root. So maybe the solution should be to add configure option for rpi to use gpio kernel driver ?

[alexvanbelle]

@rozpruwacz Sounds good.
"When I forced the build to use gpio kernel driver all works without problem when running as non root". How have you done that?

If you show me the right command, I could work on a patch on the configuration file and ask for a pull request :)

Currently, I'm using this command for configuration

 ./configure --soc=BCM2835 --my-gateway=ethernet --my-port=5003 --my-rf24-encryption-enabled --my-rf24-channel={{ rf24_channel }}

[rozpruwacz]

@alexvanbelle Well, I forced it really hard by modifying the code :) in drivers/Linux/Arduino.h in line 35 there is an ifdef that decides what to use for GPIO. For RPI case the RPi.h is included instead of GPIO.h, so I just reversed the if, this is the patch You can apply to MySensors:

diff --git a/drivers/Linux/Arduino.h b/drivers/Linux/Arduino.h
index eb9ee38..9592919 100644
--- a/drivers/Linux/Arduino.h
+++ b/drivers/Linux/Arduino.h
@@ -32,7 +32,7 @@
 #include <algorithm>
 #include "stdlib_noniso.h"
 
-#ifdef LINUX_ARCH_RASPBERRYPI
+#ifndef LINUX_ARCH_RASPBERRYPI
 #include "RPi.h"
 #define pinMode(pin, direction) RPi.pinMode(pin, direction)
 #define digitalWrite(pin, value) RPi.digitalWrite(pin, value)

[alexvanbelle]

@rozpruwacz Thanks, I will try

#ifdef LINUX_ARCH_RASPBERRYPI
#include "RPi.h"
#define pinMode(pin, direction) RPi.pinMode(pin, direction)
#define digitalWrite(pin, value) RPi.digitalWrite(pin, value)
#define digitalRead(pin) RPi.digitalRead(pin)
#define digitalPinToInterrupt(pin) RPi.digitalPinToInterrupt(pin)
#else
#include "GPIO.h"
#define pinMode(pin, direction) GPIO.pinMode(pin, direction)
#define digitalWrite(pin, value) GPIO.digitalWrite(pin, value)
#define digitalRead(pin) GPIO.digitalRead(pin)
#define digitalPinToInterrupt(pin) GPIO.digitalPinToInterrupt(pin)
#endif

By reading the code, it looks like this path is only for raspberry and, if it works, I see a proper fix such as: just remove the if and do what is in GPIO whatever the platform is.

However, I have doubts this block was added by mistake. I need to understand how automated tests would catch any side effects of this fix (regressions?). This specific code was written few years ago: therefore, maybe it was required before but is no more needed now.

Will do that:

• Test it on my setup (my raspberry gateway is connected with 10+ sensors (arduinos with nrf))
• If it works end to end, I will have to understand a little bit more how automated tests are launched on mysensors code base. If it seems OK, I will submit a PR.

[alexvanbelle]

Just for traces, this is the commit which seems to add this code block:
https://github.com/mysensors/MySensors/pull/734/commits/074f016a69fd5d67e033b6473b3e39535fd9e2d5

[alexvanbelle]

Hi,

As a follow-up:

• I tried your patch but it doesn't compile on my raspberry
• Whatever, I believe that, if it's the right problem, we might have a better solution :)

Solutions:

• I've looked to the code and it happens that, by following the calls, the final call are made to /drivers/BCM/bcm2835.h/cpp
• This library is imported from here and is currently on v1.50 in MySensors codebase. Latest release is 1.57.
• 1.50 of this library introduced the right to run as non root.
• 1.51 is fixing a bug and its description is "1.51 2016-11-03 Added documentation about SPI clock divider and resulting SPI speeds on RPi3. Fixed a problem where seg fault could occur in bcm2835_delayMicroseconds() if not running as root. Patch from Pok."

This is the related thread on the patch: https://groups.google.com/forum/#!topic/bcm2835/5VqsHKISDfQ

My best plan is then just to update the library version included in mysensors to its latest release

Will keep this thread updated.

[alexvanbelle]

Ok. Good news and bad news.

Good news:

• Updating the library is no brainer, just copy/paste and rebuild. Signatures have not changed
• After the build mysensors just works as before in root
• With non root user, you don't get any segfault.

Bad news:

• You still get an error (SPI requires root privilege => which is better than a segfault). Actually, the library doesn't require root for GPIO but it's still required for SPI and I2C.
• I've read the code of the library and some other blog posts on this subject and currently, using SPI and I2C without root seems not to be an option. The library is initializing itself by defining memory pointers => for SPI and I2C, it's undefined and there's no clear approach to set them (root and non-root init is really different for GPIO)

I still believe updating the library is good idea: cleaner error message and get some other fixes.

I have opened a thread on the library google group to get more information on the reasons: https://groups.google.com/forum/#!topic/bcm2835/i3LvA2c38j4

Alexandre

[rozpruwacz]

@alexvanbelle what is your configure options? And what is i2c used for?

[alexvanbelle]

@rozpruwacz Sorry, it's hard for me to work on this subject during week days.
I2C is in the code, I'm just trying to find a generic solution. Currently, my raspberry has only the nrf24, therefore, I'm probably not using I2C.

My configure options are:

./configure --soc=BCM2835 --my-gateway=ethernet --my-port=5003 --my-rf24-encryption-enabled --my-rf24-channel={{ rf24_channel }}

Note that since my network is only sending information (nothing is controlled), I'm not worried about not using signatures between nodes. I enabled it in the past but then, disabled it.