[PATCH 4.4 0/3] ALSA: control: Fix locking around snd_ctl_elem_read/write


Alexander Grund
 

I found a bug in v4.4-st38 caused by e8064dec769e6 "ALSA: pcm: Move rwsem lock inside snd_ctl_elem_read to prevent UAF"
The most serious part is a `down_write` call before calling `snd_ctl_elem_write` which will do a `down_read` on the same mutex causing a deadlock.
I fix this by taking missing commits from 4.14.y, especially "ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations"
which moves the locking out of those 2 functions such that the bug fixed
by the above commit is actually introduced so that it can be fixed
avoiding the deadlock.
This also requires a follow-up commit to fix a bug introduced in a
condition by that commit.
The final commit replaces `down_read` by `down_write` in `snd_ctl_elem_write_user`
similar to what e8064dec769e6 does in control_compat.c so that this use
is uniform (and correct).


Richard Fitzgerald (1):
ALSA: control: Fix memory corruption risk in snd_ctl_elem_read

Takashi Sakamoto (2):
ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations
ALSA: control: use counting semaphore as write lock for ELEM_WRITE
operation

sound/core/control.c | 78 +++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 40 deletions(-)

--
2.40.0


Pavel Machek
 

Hi!

I found a bug in v4.4-st38 caused by e8064dec769e6 "ALSA: pcm: Move
rwsem lock inside snd_ctl_elem_read to prevent UAF"
One option would be to revert e8064dec769e6.

Second option is to cherry-pick these:

Richard Fitzgerald (1):
ALSA: control: Fix memory corruption risk in snd_ctl_elem_read

Takashi Sakamoto (2):
ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations
ALSA: control: use counting semaphore as write lock for ELEM_WRITE
operation
They are in 4.14, it is probably easiest to cherry-pick them from
there.

Thanks for submission!

Best regards,
Pavel

--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Alexander Grund
 

Second option is to cherry-pick these:

That's exactly what I did here. From my submissions to the upstream kernels I was told that a 0/x EMail is required for a multi-commit change.

I haven't found a guideline how to submit patches to CIP kernels, so I just followed that procedure. See: https://lists.cip-project.org/g/cip-dev/message/11117 https://lists.cip-project.org/g/cip-dev/message/11118 https://lists.cip-project.org/g/cip-dev/message/11119

It seems that was wrong. So can you tell me please how to submit patches to CIP in the future and whether the current mails are acceptable (despite the minor description changes requested there)?

Thanks,

Alex


Ulrich Hecht
 

On 04/02/2023 6:30 PM CEST Pavel Machek <pavel@...> wrote:
Second option is to cherry-pick these:

Richard Fitzgerald (1):
ALSA: control: Fix memory corruption risk in snd_ctl_elem_read

Takashi Sakamoto (2):
ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations
ALSA: control: use counting semaphore as write lock for ELEM_WRITE
operation
They are in 4.14, it is probably easiest to cherry-pick them from
there.
They don't apply cleanly, though, so I'm happy to pick up Alexander's series.

CU
Uli