* [PATCH] ALSA: fireface: fix locking bug in ff400_copy_msg_to_user()
@ 2023-01-17 14:17 Dan Carpenter
2023-01-18 0:08 ` Takashi Sakamoto
2023-01-18 16:50 ` Takashi Iwai
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2023-01-17 14:17 UTC (permalink / raw)
To: Takashi Sakamoto
Cc: alsa-devel, Clemens Ladisch, kernel-janitors, Takashi Iwai
The ff400_copy_msg_to_user() function drops the spin lock to call
copy_to_user(). However, if the copy_to_user() fails, then it must
take the lock again before returning. Failure to take the lock leads
to a double unlock in the caller, hwdep_read().
Fixes: acdebd8b4c0c ("ALSA: fireface: implement message parser for Fireface 400")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
sound/firewire/fireface/ff-protocol-former.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/sound/firewire/fireface/ff-protocol-former.c b/sound/firewire/fireface/ff-protocol-former.c
index f58008762fe6..fa41de978756 100644
--- a/sound/firewire/fireface/ff-protocol-former.c
+++ b/sound/firewire/fireface/ff-protocol-former.c
@@ -680,28 +680,30 @@ static long ff400_copy_msg_to_user(struct snd_ff *ff, char __user *buf, long cou
struct ff400_msg_parser *parser = ff->msg_parser;
u32 type = SNDRV_FIREWIRE_EVENT_FF400_MESSAGE;
long consumed = 0;
+ int ret = 0;
if (count < 8)
return 0;
spin_unlock_irq(&ff->lock);
-
if (copy_to_user(buf, &type, sizeof(type)))
- return -EFAULT;
-
+ ret = -EFAULT;
spin_lock_irq(&ff->lock);
+ if (ret)
+ return ret;
count -= sizeof(type);
consumed += sizeof(type);
while (count >= sizeof(*parser->msgs) && parser->pull_pos != parser->push_pos) {
spin_unlock_irq(&ff->lock);
-
if (copy_to_user(buf + consumed, parser->msgs + parser->pull_pos,
sizeof(*parser->msgs)))
- return -EFAULT;
-
+ ret = -EFAULT;
spin_lock_irq(&ff->lock);
+ if (ret)
+ return ret;
+
++parser->pull_pos;
if (parser->pull_pos >= FF400_QUEUE_SIZE)
parser->pull_pos = 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ALSA: fireface: fix locking bug in ff400_copy_msg_to_user()
2023-01-17 14:17 [PATCH] ALSA: fireface: fix locking bug in ff400_copy_msg_to_user() Dan Carpenter
@ 2023-01-18 0:08 ` Takashi Sakamoto
2023-01-18 16:50 ` Takashi Iwai
1 sibling, 0 replies; 3+ messages in thread
From: Takashi Sakamoto @ 2023-01-18 0:08 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, alsa-devel, Clemens Ladisch, Takashi Iwai
Hi,
On Tue, Jan 17, 2023 at 05:17:29PM +0300, Dan Carpenter wrote:
> The ff400_copy_msg_to_user() function drops the spin lock to call
> copy_to_user(). However, if the copy_to_user() fails, then it must
> take the lock again before returning. Failure to take the lock leads
> to a double unlock in the caller, hwdep_read().
>
> Fixes: acdebd8b4c0c ("ALSA: fireface: implement message parser for Fireface 400")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> sound/firewire/fireface/ff-protocol-former.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
Indeed. Thanks for your care.
Acked-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> diff --git a/sound/firewire/fireface/ff-protocol-former.c b/sound/firewire/fireface/ff-protocol-former.c
> index f58008762fe6..fa41de978756 100644
> --- a/sound/firewire/fireface/ff-protocol-former.c
> +++ b/sound/firewire/fireface/ff-protocol-former.c
> @@ -680,28 +680,30 @@ static long ff400_copy_msg_to_user(struct snd_ff *ff, char __user *buf, long cou
> struct ff400_msg_parser *parser = ff->msg_parser;
> u32 type = SNDRV_FIREWIRE_EVENT_FF400_MESSAGE;
> long consumed = 0;
> + int ret = 0;
>
> if (count < 8)
> return 0;
>
> spin_unlock_irq(&ff->lock);
> -
> if (copy_to_user(buf, &type, sizeof(type)))
> - return -EFAULT;
> -
> + ret = -EFAULT;
> spin_lock_irq(&ff->lock);
> + if (ret)
> + return ret;
>
> count -= sizeof(type);
> consumed += sizeof(type);
>
> while (count >= sizeof(*parser->msgs) && parser->pull_pos != parser->push_pos) {
> spin_unlock_irq(&ff->lock);
> -
> if (copy_to_user(buf + consumed, parser->msgs + parser->pull_pos,
> sizeof(*parser->msgs)))
> - return -EFAULT;
> -
> + ret = -EFAULT;
> spin_lock_irq(&ff->lock);
> + if (ret)
> + return ret;
> +
> ++parser->pull_pos;
> if (parser->pull_pos >= FF400_QUEUE_SIZE)
> parser->pull_pos = 0;
> --
> 2.35.1
Regards
Takashi Sakamoto
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ALSA: fireface: fix locking bug in ff400_copy_msg_to_user()
2023-01-17 14:17 [PATCH] ALSA: fireface: fix locking bug in ff400_copy_msg_to_user() Dan Carpenter
2023-01-18 0:08 ` Takashi Sakamoto
@ 2023-01-18 16:50 ` Takashi Iwai
1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2023-01-18 16:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel, Clemens Ladisch, kernel-janitors, Takashi Iwai
On Tue, 17 Jan 2023 15:17:29 +0100,
Dan Carpenter wrote:
>
> The ff400_copy_msg_to_user() function drops the spin lock to call
> copy_to_user(). However, if the copy_to_user() fails, then it must
> take the lock again before returning. Failure to take the lock leads
> to a double unlock in the caller, hwdep_read().
>
> Fixes: acdebd8b4c0c ("ALSA: fireface: implement message parser for Fireface 400")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Thanks, applied.
Takashi
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-18 16:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-17 14:17 [PATCH] ALSA: fireface: fix locking bug in ff400_copy_msg_to_user() Dan Carpenter
2023-01-18 0:08 ` Takashi Sakamoto
2023-01-18 16:50 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox