search

stm32-rs / stm32g4xx-hal

Peripheral access API for STM32G4 series microcontrollers
Apache License 2.0
57 stars 45 forks source link

Race condition in gpio #126

Open usbalbin opened 2 months ago

usbalbin commented 2 months ago

I hope I am wrong, but I think there might be a race condition in the gpio's into_{x} methods.

https://github.com/stm32-rs/stm32g4xx-hal/blob/f8b6ff82d26d3a235d44cd0aa760cb3afd99ee35/src/gpio.rs#L386

Notice how we perform a read-modify-write sequence without anything preventing the same thing being done to another pin in the same port.

As far as I can see, I believe we would need either a critical section or exclusive access to some sort of token to guarantee exclusive access. A critical section would cause a slight bit of overhead but would otherwise be non-breaking API-wise, I think.

dtjones190 commented 1 month ago

I think that you are correct, there is a potential race condition here. It looks like the stm32g4 supports bit-banding on peripherals, so that would give you atomic writes of single bits without needing critical sections or API changes.

usbalbin commented 1 month ago

But writing the bits one at a time would on the other hand cause potentialy bad things to happen since the write would be split up into multiple writes.

For example looking at PUPD[15:0][1:0] in the RM, changing from 01: Pull-up to 10: Pull-down would be

  1. Store 1 to left bit at bit banding address <-- We now have 11 which is reserved
  2. Store 0 to right bit
usbalbin commented 1 month ago

However i suppose it would work very well for things that are single bit like OT[15:0] in GPIOx_OTYPER