apache / apache/nuttx

STM32 M0 ADC: adc_sampletime_cfg() passes generic adc_dev_s instead of dev->ad_priv

Open Beginner friendly
#19,554 2 comments 0 reactions 0 assignees View on GitHub
Arch: all Area: Drivers OS: Linux Type: Bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.