[Backport v4.4-branch] drivers: i2s: esp32: reject an unsupported trigger direction - #116530
Open
github-actions[bot] wants to merge 1 commit into
Open
[Backport v4.4-branch] drivers: i2s: esp32: reject an unsupported trigger direction#116530github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
i2s_esp32_trigger_check() validates the direction for I2S_DIR_BOTH and then does not validate it for either single direction. The BOTH branch checks all four pointers and returns -ENOSYS when the instance does not have both directions; the I2S_DIR_RX and I2S_DIR_TX branches read ->configured straight out of a stream that may not exist. The instantiation macro gives a direction the devicetree does not describe a null conf and a null data, so on an instance that enables only one direction, triggering the other one dereferences NULL rather than returning an error. Add the guard the BOTH branch already has, and return the same -ENOSYS for the same reason: the hardware this instance describes cannot do what the caller asked. -EINVAL a few lines below stays for a direction that is not one of the enumerators, which is a different mistake. Signed-off-by: Hsiu-Chi Tsai <hctsai@linux.com> (cherry picked from commit 0d82dbb)
sylvioalves
approved these changes
Aug 17, 2026
thc1006
approved these changes
Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport 0d82dbb from #115721.
Original PR description:
What this fixes
i2s_esp32_trigger_check()validates the direction forI2S_DIR_BOTHand then does not validate it for either single direction. TheI2S_DIR_BOTHbranch checks all four pointers and returns-ENOSYSwhen the instance does not have both directions; theI2S_DIR_RXandI2S_DIR_TXbranches read->configuredstraight out of a stream that may not exist.The instantiation macro gives a direction the devicetree does not describe a null
confand a nulldata, so on an instance that enables only one direction, triggering the other one dereferences NULL rather than returning an error. This adds the guard theI2S_DIR_BOTHbranch already has, and returns the same-ENOSYSfor the same reason: the hardware this instance describes cannot do what the caller asked.Fixes #115719.
Why -ENOSYS rather than -EINVAL
-EINVALis already used a few lines below for a direction value that is not one of the enumerators, which is a different mistake — a bad argument rather than a request the instance cannot serve. Keeping the two apart means a caller can tell "you passed nonsense" from "this instance has no TX", and it matches whatI2S_DIR_BOTHalready returns in exactly this situation.Is this the only place with the same hole?
I checked rather than assumed, because a guard added to one function while its siblings keep the same gap is worse than no guard at all — it reads as if the class of bug has been dealt with.
Every other dereference of
rx.dataortx.datain the file is already safe. Two of them sit behind an explicitdev_cfg->tx.data &&test. The rest are inside theI2S_DIR_BOTHbranch of this same function, or are reached only afteractive_dirhas been set toI2S_DIR_BOTH, which cannot happen unless this function first validated all four pointers — the two reads in the callbacks are short-circuited by anactive_dir == I2S_DIR_BOTHtest, and the two in the start rollback sit insideif (dir == I2S_DIR_BOTH && err < 0). So these two branches were the only unguarded ones.Verification
Built for
esp32s3_devkitc/esp32s3/procpuand foresp32_devkitc/esp32/procpu, so both the GDMA and the non-GDMA halves of this driver are covered rather than only the one my own board takes.I have not exercised the faulting path on hardware, because the board I have describes both directions and so cannot reach it. The change is a guard on a branch that was previously unguarded, and it does not alter behaviour for an instance that has the direction being triggered.