lowRISC / lowRISC/opentitan

[aes] dif_aes_trigger does not check manual mode or data loaded

Open
#30,533 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### Description

While reviewing the AES DIF against its RTL specification
(OpenTitan @ ffab8da7381c66b3d2e2b78a382a9f6937e5482e), we noticed that
`dif_aes_trigger` writes the TRIGGER register without verifying that
the AES module is in manual mode and that data has been loaded.

**Document constraint** (`dif_aes.h` or RTL spec):
> "Trigger should only be used in manual mode after data is loaded"

**Current behavior**:
`dif_aes_trigger` (in `dif_aes.c`) directly writes the TRIGGER register
without checking:
1. Whether the AES module is in manual mode
2. Whether data has been loaded into the module

**Impact**:
Triggering in automatic mode or before data is loaded may cause
undefined behavior or incorrect encryption results. The DIF should
enforce this constraint at the API boundary to prevent callers from
inadvertently triggering the AES module in an invalid state.

**Note**:
The AES DIF otherwise demonstrates high-quality defensive programming
(e.g., `dif_aes_start` checks IDLE status, `dif_aes_load_data` checks
INPUT_READY, `dif_aes_read_output` checks OUTPUT_VALID). This appears
to be a boundary oversight in the trigger function.

**Suggested fix**:
Add checks in `dif_aes_trigger` to verify:
- `STATUS.IDLE = 0` (module is active/busy, implying data is loaded)
- Or `CTRL.MANUAL_OPERATION = 1` (manual mode is enabled)

Return `kDifError` if the preconditions are not met.

## Version
- OpenTitan commit: ffab8da7381c66b3d2e2b78a382a9f6937e5482e
- AES verification: V2, DIF stage: S2

Contributor guide

Open the contributing guide

Research direction

Start in dif_aes.c at dif_aes_trigger and read the related API checks in the same DIF, then compare the precondition in dif_aes.h with the referenced RTL specification. Determine the required manual-mode and data-loaded state checks, return kDifError when they are not satisfied, and verify that valid triggers still write the TRIGGER register.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
cryptography, embedded-iot
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.