* [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel
@ 2022-04-14 18:31 Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH 1/2] ALSA: core: Add snd_card_free_on_error() helper Nirmoy Das
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Nirmoy Das @ 2022-04-14 18:31 UTC (permalink / raw
To: intel-gfx
Run CI for these two patches for:
https://gitlab.freedesktop.org/drm/intel/-/issues/5701
Nirmoy Das (1):
ALSA: hda: handle UAF at probe error
Takashi Iwai (1):
ALSA: core: Add snd_card_free_on_error() helper
include/sound/core.h | 1 +
sound/core/init.c | 28 ++++++++++++++++++++++++++++
sound/pci/hda/hda_intel.c | 7 ++++---
3 files changed, 33 insertions(+), 3 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-gfx] [PATCH 1/2] ALSA: core: Add snd_card_free_on_error() helper
2022-04-14 18:31 [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel Nirmoy Das
@ 2022-04-14 18:31 ` Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH v2] ALSA: hda: handle UAF at probe error Nirmoy Das
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nirmoy Das @ 2022-04-14 18:31 UTC (permalink / raw
To: intel-gfx
From: Takashi Iwai <tiwai@suse.de>
This is a small helper function to handle the error path more easily
when an error happens during the probe for the device with the
device-managed card. Since devres releases in the reverser order of
the creations, usually snd_card_free() gets called at the last in the
probe error path unless it already reached snd_card_register() calls.
Due to this nature, when a driver expects the resource releases in
card->private_free, this might be called too lately.
As a workaround, one should call the probe like:
static int __some_probe(...) { // do real probe.... }
static int some_probe(...)
{
return snd_card_free_on_error(dev, __some_probe(dev, ...));
}
so that the snd_card_free() is called explicitly at the beginning of
the error path from the probe.
This function will be used in the upcoming fixes to address the
regressions by devres usages.
Fixes: e8ad415b7a55 ("ALSA: core: Add managed card creation")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20220412093141.8008-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
include/sound/core.h | 1 +
sound/core/init.c | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/include/sound/core.h b/include/sound/core.h
index b7e9b58d3c78..6d4cc49584c6 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -284,6 +284,7 @@ int snd_card_disconnect(struct snd_card *card);
void snd_card_disconnect_sync(struct snd_card *card);
int snd_card_free(struct snd_card *card);
int snd_card_free_when_closed(struct snd_card *card);
+int snd_card_free_on_error(struct device *dev, int ret);
void snd_card_set_id(struct snd_card *card, const char *id);
int snd_card_register(struct snd_card *card);
int snd_card_info_init(void);
diff --git a/sound/core/init.c b/sound/core/init.c
index 31ba7024e3ad..726a8353201f 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -209,6 +209,12 @@ static void __snd_card_release(struct device *dev, void *data)
* snd_card_register(), the very first devres action to call snd_card_free()
* is added automatically. In that way, the resource disconnection is assured
* at first, then released in the expected order.
+ *
+ * If an error happens at the probe before snd_card_register() is called and
+ * there have been other devres resources, you'd need to free the card manually
+ * via snd_card_free() call in the error; otherwise it may lead to UAF due to
+ * devres call orders. You can use snd_card_free_on_error() helper for
+ * handling it more easily.
*/
int snd_devm_card_new(struct device *parent, int idx, const char *xid,
struct module *module, size_t extra_size,
@@ -235,6 +241,28 @@ int snd_devm_card_new(struct device *parent, int idx, const char *xid,
}
EXPORT_SYMBOL_GPL(snd_devm_card_new);
+/**
+ * snd_card_free_on_error - a small helper for handling devm probe errors
+ * @dev: the managed device object
+ * @ret: the return code from the probe callback
+ *
+ * This function handles the explicit snd_card_free() call at the error from
+ * the probe callback. It's just a small helper for simplifying the error
+ * handling with the managed devices.
+ */
+int snd_card_free_on_error(struct device *dev, int ret)
+{
+ struct snd_card *card;
+
+ if (!ret)
+ return 0;
+ card = devres_find(dev, __snd_card_release, NULL, NULL);
+ if (card)
+ snd_card_free(card);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(snd_card_free_on_error);
+
static int snd_card_init(struct snd_card *card, struct device *parent,
int idx, const char *xid, struct module *module,
size_t extra_size)
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-gfx] [PATCH v2] ALSA: hda: handle UAF at probe error
2022-04-14 18:31 [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH 1/2] ALSA: core: Add snd_card_free_on_error() helper Nirmoy Das
@ 2022-04-14 18:31 ` Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH 2/2] " Nirmoy Das
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nirmoy Das @ 2022-04-14 18:31 UTC (permalink / raw
To: intel-gfx
Call snd_card_free_on_error() on probe error instead of
calling snd_card_free() which should handle devres call orders.
Issues: https://gitlab.freedesktop.org/drm/intel/-/issues/5701
Fixes: e8ad415b7a55 ("ALSA: core: Add managed card creation")
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
sound/pci/hda/hda_intel.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 82a45f2b31c4..eb4228c9e37f 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1730,7 +1730,9 @@ static void azx_check_snoop_available(struct azx *chip)
static void azx_probe_work(struct work_struct *work)
{
struct hda_intel *hda = container_of(work, struct hda_intel, probe_work.work);
- azx_probe_continue(&hda->chip);
+ struct azx *chip = &hda->chip;
+
+ snd_card_free_on_error(&chip->pci->dev, azx_probe_continue(chip));
}
static int default_bdl_pos_adj(struct azx *chip)
@@ -2028,7 +2030,7 @@ static void azx_firmware_cb(const struct firmware *fw, void *context)
dev_err(card->dev, "Cannot load firmware, continue without patching\n");
if (!chip->disabled) {
/* continue probing */
- azx_probe_continue(chip);
+ snd_card_free_on_error(&chip->pci->dev, azx_probe_continue(chip));
}
}
#endif
@@ -2338,7 +2340,6 @@ static int azx_probe_continue(struct azx *chip)
out_free:
if (err < 0) {
pci_set_drvdata(pci, NULL);
- snd_card_free(chip->card);
return err;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-gfx] [PATCH 2/2] ALSA: hda: handle UAF at probe error
2022-04-14 18:31 [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH 1/2] ALSA: core: Add snd_card_free_on_error() helper Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH v2] ALSA: hda: handle UAF at probe error Nirmoy Das
@ 2022-04-14 18:31 ` Nirmoy Das
2022-04-14 22:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-04-14 23:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
4 siblings, 0 replies; 6+ messages in thread
From: Nirmoy Das @ 2022-04-14 18:31 UTC (permalink / raw
To: intel-gfx
Call snd_card_free_on_error() on probe error instead of
calling snd_card_free() which should handle devres call orders.
Issues: https://gitlab.freedesktop.org/drm/intel/-/issues/5701
Fixes: e8ad415b7a55 ("ALSA: core: Add managed card creation")
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
sound/pci/hda/hda_intel.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 82a45f2b31c4..eb4228c9e37f 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1730,7 +1730,9 @@ static void azx_check_snoop_available(struct azx *chip)
static void azx_probe_work(struct work_struct *work)
{
struct hda_intel *hda = container_of(work, struct hda_intel, probe_work.work);
- azx_probe_continue(&hda->chip);
+ struct azx *chip = &hda->chip;
+
+ snd_card_free_on_error(&chip->pci->dev, azx_probe_continue(chip));
}
static int default_bdl_pos_adj(struct azx *chip)
@@ -2028,7 +2030,7 @@ static void azx_firmware_cb(const struct firmware *fw, void *context)
dev_err(card->dev, "Cannot load firmware, continue without patching\n");
if (!chip->disabled) {
/* continue probing */
- azx_probe_continue(chip);
+ snd_card_free_on_error(&chip->pci->dev, azx_probe_continue(chip));
}
}
#endif
@@ -2338,7 +2340,6 @@ static int azx_probe_continue(struct azx *chip)
out_free:
if (err < 0) {
pci_set_drvdata(pci, NULL);
- snd_card_free(chip->card);
return err;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for ALSA: hda: handle UAF at probe error
2022-04-14 18:31 [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel Nirmoy Das
` (2 preceding siblings ...)
2022-04-14 18:31 ` [Intel-gfx] [PATCH 2/2] " Nirmoy Das
@ 2022-04-14 22:56 ` Patchwork
2022-04-14 23:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
4 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2022-04-14 22:56 UTC (permalink / raw
To: Nirmoy Das; +Cc: intel-gfx
== Series Details ==
Series: ALSA: hda: handle UAF at probe error
URL : https://patchwork.freedesktop.org/series/102714/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i2c/tda998x_drv.c:1712:50: expected restricted __be32 const [usertype] *p
+drivers/gpu/drm/i2c/tda998x_drv.c:1712:50: got unsigned int const [usertype] *
+drivers/gpu/drm/i2c/tda998x_drv.c:1712:50: warning: incorrect type in argument 1 (different base types)
+drivers/gpu/drm/i2c/tda998x_drv.c:1713:52: expected restricted __be32 const [usertype] *p
+drivers/gpu/drm/i2c/tda998x_drv.c:1713:52: got unsigned int const [usertype] *
+drivers/gpu/drm/i2c/tda998x_drv.c:1713:52: warning: incorrect type in argument 1 (different base types)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for ALSA: hda: handle UAF at probe error
2022-04-14 18:31 [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel Nirmoy Das
` (3 preceding siblings ...)
2022-04-14 22:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
@ 2022-04-14 23:18 ` Patchwork
4 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2022-04-14 23:18 UTC (permalink / raw
To: Nirmoy Das; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7123 bytes --]
== Series Details ==
Series: ALSA: hda: handle UAF at probe error
URL : https://patchwork.freedesktop.org/series/102714/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11503 -> Patchwork_102714v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_102714v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_102714v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/index.html
Participating hosts (49 -> 45)
------------------------------
Missing (4): fi-kbl-soraka fi-bdw-samus fi-bsw-cyan bat-adlp-4
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_102714v1:
### IGT changes ###
#### Possible regressions ####
* igt@i915_suspend@system-suspend-without-i915:
- fi-tgl-1115g4: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/fi-tgl-1115g4/igt@i915_suspend@system-suspend-without-i915.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-tgl-1115g4/igt@i915_suspend@system-suspend-without-i915.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@gem_lmem_swapping@basic:
- {bat-rpls-2}: NOTRUN -> [FAIL][3] +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/bat-rpls-2/igt@gem_lmem_swapping@basic.html
* igt@i915_module_load@reload:
- {bat-rpls-2}: [INCOMPLETE][4] ([i915#5329]) -> [DMESG-WARN][5]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/bat-rpls-2/igt@i915_module_load@reload.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/bat-rpls-2/igt@i915_module_load@reload.html
* igt@i915_pm_rpm@module-reload:
- {fi-ehl-2}: [PASS][6] -> [SKIP][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/fi-ehl-2/igt@i915_pm_rpm@module-reload.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-ehl-2/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live:
- {fi-ehl-2}: NOTRUN -> [SKIP][8]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-ehl-2/igt@i915_selftest@live.html
* igt@i915_suspend@system-suspend-without-i915:
- {fi-ehl-2}: [PASS][9] -> [FAIL][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/fi-ehl-2/igt@i915_suspend@system-suspend-without-i915.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-ehl-2/igt@i915_suspend@system-suspend-without-i915.html
Known issues
------------
Here are the changes found in Patchwork_102714v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_pm_rpm@module-reload:
- fi-tgl-1115g4: [PASS][11] -> [FAIL][12] ([i915#579])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live:
- fi-tgl-1115g4: NOTRUN -> [SKIP][13] ([i915#1245])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-tgl-1115g4/igt@i915_selftest@live.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-g3258: [PASS][14] -> [INCOMPLETE][15] ([i915#4785])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
* igt@runner@aborted:
- fi-hsw-g3258: NOTRUN -> [FAIL][16] ([fdo#109271] / [i915#4312])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-hsw-g3258/igt@runner@aborted.html
#### Possible fixes ####
* igt@i915_suspend@system-suspend-without-i915:
- {fi-tgl-dsi}: [DMESG-WARN][17] ([i915#1982]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/fi-tgl-dsi/igt@i915_suspend@system-suspend-without-i915.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/fi-tgl-dsi/igt@i915_suspend@system-suspend-without-i915.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
[i915#1245]: https://gitlab.freedesktop.org/drm/intel/issues/1245
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
[i915#5329]: https://gitlab.freedesktop.org/drm/intel/issues/5329
[i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341
[i915#5401]: https://gitlab.freedesktop.org/drm/intel/issues/5401
[i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
Build changes
-------------
* Linux: CI_DRM_11503 -> Patchwork_102714v1
CI-20190529: 20190529
CI_DRM_11503: 000a595e443e99065d0ea00993a60eef24276e5f @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6437: ea0144ed6ccb66b977f204b4d53b6062ed1cc8bc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_102714v1: 000a595e443e99065d0ea00993a60eef24276e5f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
c287159b48b5 ALSA: hda: handle UAF at probe error
5e85240f9606 ALSA: core: Add snd_card_free_on_error() helper
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102714v1/index.html
[-- Attachment #2: Type: text/html, Size: 6755 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-14 23:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-14 18:31 [Intel-gfx] [CI 0/2] Fix UAF in snd_hda_intel Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH 1/2] ALSA: core: Add snd_card_free_on_error() helper Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH v2] ALSA: hda: handle UAF at probe error Nirmoy Das
2022-04-14 18:31 ` [Intel-gfx] [PATCH 2/2] " Nirmoy Das
2022-04-14 22:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-04-14 23:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.