STM32 M0 ADC: adc_sampletime_cfg() passes generic adc_dev_s instead of dev->ad_priv
- Dominant language
- C
- Stars
- 4k
- Forks
- 1.7k
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 237
Description
### Description / Steps to reproduce the issue
`adc_sampletime_cfg()` in `arch/arm/src/common/stm32/stm32_adc_m0_v1.c` passes the generic ADC object, `struct adc_dev_s *dev`, directly to the sample-time helper functions. Both helpers then cast the received pointer back to `struct stm32_dev_s *`.
However, `g_adcdev1` and `g_adcpriv1` are separate objects. As a result, the sample-time values are written into the wrong object, and the register base address is also read from the wrong object.
## Problematic Code
Lines 1461–1462 of `stm32_adc_m0_v1.c`, in the branch enabled by `CONFIG_STM32_ADC_CHANGE_SAMPLETIME`, contain the following calls:
```c
adc_sampletime_set((struct stm32_adc_dev_s *)dev, &time_samples);
adc_sampletime_write((struct stm32_adc_dev_s *)dev);
```
The two called functions cast the argument back to the private type at lines 2754 and 2791:
```c
static void adc_sampletime_set(struct stm32_adc_dev_s *dev,
struct adc_sample_time_s *time_samples)
{
struct stm32_dev_s *priv = (struct stm32_dev_s *)dev;
priv->sample_rate[0] = time_samples->smp1;
...
}
```
The `#else` branch of the same function, at line 1464, already retrieves the private object correctly through `dev->ad_priv`:
```c
struct stm32_dev_s *priv = (struct stm32_dev_s *)dev->ad_priv;
```
## Field-to-Field Mapping
The following table shows which locations in `struct adc_dev_s` are accessed when fields of `struct stm32_dev_s` are referenced through the incorrect pointer:
| `struct stm32_dev_s` field | Offset | Corresponding location in `struct adc_dev_s` |
|---|---:|---|
| `sample_rate[0..1]` | 20–21 | `ad_lock` |
| `smpsel` | 24 | `ad_lock` |
| `base` read | 32 | `ad_recvsem` |
Therefore, the sample-time values overwrite synchronization-related fields in the generic ADC object, while part of `ad_recvsem` is interpreted as the peripheral register base address.
## Reproduction Procedure
1. Configure the in-tree ADC DMA configuration:
```sh
./tools/configure.sh -l nucleo-g0b1re:adc_dma
```
2. Build NuttX:
```sh
make -j8
```
3. Execute the generated ELF's Cortex-M0 code under Unicorn, starting from `adc_sampletime_cfg()`.
The observed results are shown below.
```text
g_adcdev1 = 0x2000011c
g_adcpriv1 = 0x200000e0
[Unmodified upstream code]
Modified offsets in g_adcdev1: [20, 21]
Modified offsets in g_adcpriv1: []
Base read from g_adcdev1+32: 0x00000000
Attempted register write: 0x00000014
[Synchronization fields initialized with marker values]
Modified offsets in g_adcdev1: [20, 21, 24, 25, 26, 27]
Modified offsets in g_adcpriv1: []
Base read from g_adcdev1+32: 0xb6b6b6b6
Attempted register write: 0xb6b6b6ca
[Control: adc_sampletime_cfg(g_adcpriv1)]
Modified offsets in g_adcdev1: []
Modified offsets in g_adcpriv1: [20, 21, 24, 25, 26, 27]
Base read from g_adcpriv1+32: 0x40012400
Attempted register write: 0x40012414
```
The expected destination is:
```text
STM32_ADC1_BASE + STM32_ADC_SMPR_OFFSET
= 0x40012400 + 0x14
= 0x40012414
```
Instead, the unmodified code attempts to write to `0x00000014`.
The second execution initializes the synchronization fields with marker values to demonstrate the origin of the corrupted base address explicitly. The third execution is a control run in which the correct private pointer is passed.
## Comparison with Sibling Drivers
At the equivalent call site, other STM32-family ADC drivers pass `dev->ad_priv` rather than the generic ADC object:
- `stm32_adc_m3m4_v1v2.c:2571`
- `stm32f7/stm32_adc.c:1591`
- `at32/at32_adc.c:2112`
Only the M0 variant passes the generic pointer, which appears to deviate from the intended driver pattern.
## Impact
In builds where `CONFIG_STM32_ADC_CHANGE_SAMPLETIME=y`, the issue is triggered when the ADC device is opened for the first time through the following call path:
```text
adc_setup()
-> adc_configure()
-> adc_sampletime_cfg()
-> adc_sampletime_set()
-> adc_sampletime_write()
-> adc_putreg()
```
Among the in-tree configurations, `nucleo-g0b1re:adc_dma` enables this option.
The issue corrupts the mutex or synchronization state stored in the generic ADC object and causes an MMIO write to an address derived from unrelated synchronization-object contents.
## Proposed Fix
The private object can be retrieved before invoking the helper functions:
```c
struct stm32_dev_s *priv = (struct stm32_dev_s *)dev->ad_priv;
adc_sampletime_set((struct stm32_adc_dev_s *)priv, &time_samples);
adc_sampletime_write((struct stm32_adc_dev_s *)priv);
```
Alternatively, the helper function signatures could be changed to accept `struct stm32_dev_s *` directly, eliminating the intermediate casts entirely.
## Additional Context
I initially reported this issue to `security@apache.org`.
ASF Security confirmed the defect in the following `master` revision:
```text
e35129b98d79885f31c6bcbaac5cfe3142377e3f
```
They also confirmed that the sibling drivers pass `dev->ad_priv` at the corresponding call sites. However, because the issue is considered a functional defect rather than a vulnerability under the NuttX security model, they advised reporting it through the normal public channel.
### On which OS does this issue occur?
[OS: Linux]
### What is the version of your OS?
Ubuntu 24.04.3 LTS
### NuttX Version
master
### Issue Architecture
[Arch: all]
### Issue Area
[Area: Drivers]
### Host information
_No response_
### Verification
- [x] I have verified before submitting the report.
### Contribution
Ijae Kim ([ijk5201@psu.edu](mailto:ijk5201@psu.edu))
Myeonghun Pak ([mhun512@gmail.com](mailto:mhun512@gmail.com))
Yuho Choi ([yqc5929@psu.edu](mailto:yqc5929@psu.edu))
Taegyu Kim ([tgkim@psu.edu](mailto:tgkim@psu.edu))
Contributor guide
Research direction
Start in arch/arm/src/common/stm32/stm32_adc_m0_v1.c at adc_sampletime_cfg(), then follow adc_sampletime_set() and adc_sampletime_write() under CONFIG_STM32_ADC_CHANGE_SAMPLETIME. Build the nucleo-g0b1re:adc_dma configuration with ./tools/configure.sh and make -j8, then reproduce the Unicorn check if available. Done means the private ADC object supplies the sample-time state and the write targets the expected STM32 ADC register without corrupting generic synchronization fields.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100