| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240408101803.43183-4-rf@opensource.cirrus.com>
Date: Mon, 8 Apr 2024 11:18:02 +0100
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: <broonie@...nel.org>, <tiwai@...e.com>
CC: <linux-sound@...r.kernel.org>, <alsa-devel@...a-project.org>,
<linux-kernel@...r.kernel.org>, <patches@...nsource.cirrus.com>,
"Richard
Fitzgerald" <rf@...nsource.cirrus.com>
Subject: [PATCH 3/4] ASoC: cs35l56: Fix unintended bus access while resetting amp
Use the new regmap_read_bypassed() so that the regmap can be left
in cache-only mode while it is booting, but the driver can still
read boot-status and chip-id information during this time.
This fixes race conditions where some writes could be issued to the
silicon while it is still rebooting, before the driver has determined
that the boot is complete.
This is typically prevented by putting regmap into cache-only until the
hardware is ready. But this assumes that the driver does not need to
access device registers to determine when it is "ready". For cs35l56
this involves polling a register and the original implementation relied
on having special handlers to block racing callbacks until dsp_work()
is complete. However, some cases were missed, most notably the ASP DAI
functions.
The regmap_read_bypassed() function allows the fix for this to be
simplified to putting regmap into cache-only during the reset. The
initial boot stages (poll HALO_STATE and read the chip ID) are all done
bypassed. Only when the amp is seen to be booted is the cache-only
revoked.
Changes are:
- cs35l56_system_reset() now leaves the regmap in cache-only status.
- cs35l56_wait_for_firmware_boot() polls using regmap_read_bypassed().
- cs35l56_init() revokes cache-only either via cs35l56_hw_init() or
when firmware has rebooted after a soft reset.
- cs35l56_hw_init() exits cache-only after it has determined that the
amp has booted.
- cs35l56_sdw_init() doesn't disable cache-only, since this must be
deferred to cs35l56_init().
- cs35l56_runtime_resume_common() waits for firmware boot before exiting
cache-only.
These changes cover three situations where the registers are not
accessible:
1) SoundWire first-time enumeration. The regmap is kept in cache-only
until the chip is fully booted. The original code had to exit
cache-only to read chip status in cs35l56_init() and cs35l56_hw_init()
but this is now deferred to after the firmware has rebooted.
In this case cs35l56_sdw_probe() leaves regmap in cache-only
(unchanged behaviour) and cs35l56_hw_init() exits cache-only after the
firmware is booted and the chip identified.
2) Soft reset during first-time initialization. cs35l56_init() calls
cs35l56_system_reset(), which puts regmap into cache-only.
On I2C/SPI cs35l56_init() then flows through to call
cs35l56_wait_for_firmware_boot() and exit cache-only. On SoundWire
the re-enumeration will enter cs35l56_init() again, which then drops
down to call cs35l56_wait_for_firmware_boot() and exit cache-only.
3) Soft reset after firmware download. dsp_work() calls
cs35l56_system_reset(), which puts regmap into cache-only. After this
the flow is the same as (2).
Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
Fixes: 8a731fd37f8b ("ASoC: cs35l56: Move utility functions to shared file")
---
sound/soc/codecs/cs35l56-sdw.c | 2 --
sound/soc/codecs/cs35l56-shared.c | 20 ++++++++++++--------
sound/soc/codecs/cs35l56.c | 2 ++
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/cs35l56-sdw.c b/sound/soc/codecs/cs35l56-sdw.c
index 14a5f86019aa..70ff55c1517f 100644
--- a/sound/soc/codecs/cs35l56-sdw.c
+++ b/sound/soc/codecs/cs35l56-sdw.c
@@ -188,8 +188,6 @@ static void cs35l56_sdw_init(struct sdw_slave *peripheral)
goto out;
}
- regcache_cache_only(cs35l56->base.regmap, false);
-
ret = cs35l56_init(cs35l56);
if (ret < 0) {
regcache_cache_only(cs35l56->base.regmap, true);
diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
index 08cac58e3ab2..a83317db75ed 100644
--- a/sound/soc/codecs/cs35l56-shared.c
+++ b/sound/soc/codecs/cs35l56-shared.c
@@ -307,10 +307,10 @@ int cs35l56_wait_for_firmware_boot(struct cs35l56_base *cs35l56_base)
reg = CS35L56_DSP1_HALO_STATE;
/*
- * This can't be a regmap_read_poll_timeout() because cs35l56 will NAK
- * I2C until it has booted which would terminate the poll
+ * The regmap must remain in cache-only until the chip has
+ * booted, so use a bypassed read of the status register.
*/
- poll_ret = read_poll_timeout(regmap_read, read_ret,
+ poll_ret = read_poll_timeout(regmap_read_bypassed, read_ret,
(val < 0xFFFF) && (val >= CS35L56_HALO_STATE_BOOT_DONE),
CS35L56_HALO_STATE_POLL_US,
CS35L56_HALO_STATE_TIMEOUT_US,
@@ -362,7 +362,8 @@ void cs35l56_system_reset(struct cs35l56_base *cs35l56_base, bool is_soundwire)
return;
cs35l56_wait_control_port_ready();
- regcache_cache_only(cs35l56_base->regmap, false);
+
+ /* Leave in cache-only. This will be revoked when the chip has rebooted. */
}
EXPORT_SYMBOL_NS_GPL(cs35l56_system_reset, SND_SOC_CS35L56_SHARED);
@@ -577,14 +578,14 @@ int cs35l56_runtime_resume_common(struct cs35l56_base *cs35l56_base, bool is_sou
cs35l56_issue_wake_event(cs35l56_base);
out_sync:
- regcache_cache_only(cs35l56_base->regmap, false);
-
ret = cs35l56_wait_for_firmware_boot(cs35l56_base);
if (ret) {
dev_err(cs35l56_base->dev, "Hibernate wake failed: %d\n", ret);
goto err;
}
+ regcache_cache_only(cs35l56_base->regmap, false);
+
ret = cs35l56_mbox_send(cs35l56_base, CS35L56_MBOX_CMD_PREVENT_AUTO_HIBERNATE);
if (ret)
goto err;
@@ -757,7 +758,7 @@ int cs35l56_hw_init(struct cs35l56_base *cs35l56_base)
* devices so the REVID needs to be determined before waiting for the
* firmware to boot.
*/
- ret = regmap_read(cs35l56_base->regmap, CS35L56_REVID, &revid);
+ ret = regmap_read_bypassed(cs35l56_base->regmap, CS35L56_REVID, &revid);
if (ret < 0) {
dev_err(cs35l56_base->dev, "Get Revision ID failed\n");
return ret;
@@ -768,7 +769,7 @@ int cs35l56_hw_init(struct cs35l56_base *cs35l56_base)
if (ret)
return ret;
- ret = regmap_read(cs35l56_base->regmap, CS35L56_DEVID, &devid);
+ ret = regmap_read_bypassed(cs35l56_base->regmap, CS35L56_DEVID, &devid);
if (ret < 0) {
dev_err(cs35l56_base->dev, "Get Device ID failed\n");
return ret;
@@ -787,6 +788,9 @@ int cs35l56_hw_init(struct cs35l56_base *cs35l56_base)
cs35l56_base->type = devid & 0xFF;
+ /* Silicon is now identified and booted so exit cache-only */
+ regcache_cache_only(cs35l56_base->regmap, false);
+
ret = regmap_read(cs35l56_base->regmap, CS35L56_DSP_RESTRICT_STS1, &secured);
if (ret) {
dev_err(cs35l56_base->dev, "Get Secure status failed\n");
diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index 8d2f021fb362..5a4e0e479414 100644
--- a/sound/soc/codecs/cs35l56.c
+++ b/sound/soc/codecs/cs35l56.c
@@ -1531,6 +1531,8 @@ int cs35l56_init(struct cs35l56_private *cs35l56)
return ret;
dev_dbg(cs35l56->base.dev, "Firmware rebooted after soft reset\n");
+
+ regcache_cache_only(cs35l56->base.regmap, false);
}
/* Disable auto-hibernate so that runtime_pm has control */
--
2.39.2
Powered by blists - more mailing lists