| Message ID | 20220124153253.3548853-1-broonie@kernel.org |
|---|---|
| Headers | show |
| Series | ASoC: Add bounds checking for written values | expand |
On 24. 01. 22 18:08, Takashi Iwai wrote: > On Mon, 24 Jan 2022 17:52:46 +0100, > Mark Brown wrote: >> >> On Mon, Jan 24, 2022 at 05:29:50PM +0100, Jaroslav Kysela wrote: >>> On 24. 01. 22 16:32, Mark Brown wrote: >> >>>> This series adds verification that values written to registers are in >>>> bounds for controls since the core doesn't validate for us. >> >>> As discussed, those conditions should be optional to eventually catch the >>> wrong applications. I don't see any benefit to report the range error back >>> when there is value masking code already. The users will note when the >>> unwanted values are written to the hardware, or not? >> >> In general I'd say that silent failures are harder to work with than >> returning an error at the point where we notice that there's a problem, >> assuming userspace is paying any attention to the error return at all of >> course. We certainly have quite a lot of existing put() methods that do >> return errors and it's not like the application isn't invoking undefined >> behaviour so I don't see a problem here. > > I find also it's more useful to have the proper checks in general. > > Jaroslav, is you concern only about the compatibility of user-space? > Or anything else? The compatibility is always certainly a slight > issue; if this breaks really something useful and actually working > stuff, we need to consider the workaround... My concern is only based on the fact that this code is normally not reachable. It only costs some CPU ticks and adds extra code to the drivers without any other benefit. The applications should not use out of range values and if they do, the behavior is not defined (the drivers should only avoid crashes). The code seems to be added only to make things consistent for the test applications. I don't think that it's worth to care only for this reason. What is the goal for this code? The result with the proposed code will be - the SoC core has the extra validation conditions. The user space can eventually add similar checks to detect the broken applications. Basically, I think that those checks should be marked as debug and they should be optional like we do for CONFIG_SND_CTL_VALIDATION and enable only the fast path by default. Jaroslav
On Mon, 24 Jan 2022 15:32:50 +0000, Mark Brown wrote: > This series adds verification that values written to registers are in > bounds for controls since the core doesn't validate for us. > > Mark Brown (3): > ASoC: ops: Reject out of bounds values in snd_soc_put_volsw() > ASoC: ops: Reject out of bounds values in snd_soc_put_volsw_sx() > ASoC: ops: Reject out of bounds values in snd_soc_put_xr_sx() > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus Thanks! [1/3] ASoC: ops: Reject out of bounds values in snd_soc_put_volsw() commit: 817f7c9335ec01e0f5e8caffc4f1dcd5e458a4c0 [2/3] ASoC: ops: Reject out of bounds values in snd_soc_put_volsw_sx() commit: 4f1e50d6a9cf9c1b8c859d449b5031cacfa8404e [3/3] ASoC: ops: Reject out of bounds values in snd_soc_put_xr_sx() commit: 4cf28e9ae6e2e11a044be1bcbcfa1b0d8675fe4d All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark