2023-11-02 19:06:59

by Philipp Stanner

[permalink] [raw]
Subject: [PATCH] sound/isa/wavefront: copy userspace array safely

wavefront_fx.c utilizes memdup_user() to copy a userspace array. This
does not check for an overflow.

Use the new wrapper memdup_array_user() to copy the array more safely.

Suggested-by: Dave Airlie <[email protected]>
Signed-off-by: Philipp Stanner <[email protected]>
---
sound/isa/wavefront/wavefront_fx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/isa/wavefront/wavefront_fx.c b/sound/isa/wavefront/wavefront_fx.c
index 3c21324b2a0e..0273b7dfaf12 100644
--- a/sound/isa/wavefront/wavefront_fx.c
+++ b/sound/isa/wavefront/wavefront_fx.c
@@ -191,9 +191,9 @@ snd_wavefront_fx_ioctl (struct snd_hwdep *sdev, struct file *file,
"> 512 bytes to FX\n");
return -EIO;
}
- page_data = memdup_user((unsigned char __user *)
- r.data[3],
- r.data[2] * sizeof(short));
+ page_data = memdup_array_user((unsigned char __user *)
+ r.data[3],
+ r.data[2], sizeof(short));
if (IS_ERR(page_data))
return PTR_ERR(page_data);
pd = page_data;
--
2.41.0


2023-11-03 13:58:49

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] sound/isa/wavefront: copy userspace array safely

On Thu, 02 Nov 2023 20:03:10 +0100,
Philipp Stanner wrote:
>
> wavefront_fx.c utilizes memdup_user() to copy a userspace array. This
> does not check for an overflow.

There is a check above the memdup_user() call; it's at most 512
bytes.

> Use the new wrapper memdup_array_user() to copy the array more safely.
>
> Suggested-by: Dave Airlie <[email protected]>
> Signed-off-by: Philipp Stanner <[email protected]>

Although the check is already present, it's still better to use the
new helper, so I applied the patch now.


thanks,

Takashi

2023-11-03 14:06:50

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] sound/isa/wavefront: copy userspace array safely

On Fri, 03 Nov 2023 14:58:22 +0100,
Takashi Iwai wrote:
>
> On Thu, 02 Nov 2023 20:03:10 +0100,
> Philipp Stanner wrote:
> >
> > wavefront_fx.c utilizes memdup_user() to copy a userspace array. This
> > does not check for an overflow.
>
> There is a check above the memdup_user() call; it's at most 512
> bytes.
>
> > Use the new wrapper memdup_array_user() to copy the array more safely.
> >
> > Suggested-by: Dave Airlie <[email protected]>
> > Signed-off-by: Philipp Stanner <[email protected]>
>
> Although the check is already present, it's still better to use the
> new helper, so I applied the patch now.

... and the helper is available only on Linus tree for now, so I
postpone after 6.7-rc1 release, so that we can have a solid base.


Takashi