On Thu, Jul 12, 2018 at 3:47 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Jul 13, 2018 at 12:29:36AM +0200, Jann Horn wrote:
> > From: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
> >
> > From: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
> >
> > If softsynthx_read() is called with `count < 3`, `count - 3` wraps, causing
> > the loop to copy as much data as available to the provided buffer. If
> > softsynthx_read() is invoked through sys_splice(), this causes an
> > unbounded kernel write; but even when userspace just reads from it
> > normally, a small size could cause userspace crashes.
>
> Or you could try this (completely untested, though):
I think this has the same problem as my original buggy patch: At the
point where you notice that you'd overflow the buffer, you've already
consumed a character from the synth buffer. You'd have to put it back,
and since the spinlock protecting it has been dropped, that's a bit
weird.
Also, I'm not sure whether Greg prefers fixes for stable kernels that
don't also contain cleanup?
> diff --git a/drivers/staging/speakup/speakup_soft.c b/drivers/staging/speakup/speakup_soft.c
> index a61bc41b82d7..198936ed0b54 100644
> --- a/drivers/staging/speakup/speakup_soft.c
> +++ b/drivers/staging/speakup/speakup_soft.c
[...]
> @@ -224,11 +221,14 @@ static ssize_t softsynthx_read(struct file *fp, char __user *buf, size_t count,
> }
> finish_wait(&speakup_event, &wait);
>
> - cp = buf;
> init = get_initstring();
>
> /* Keep 3 bytes available for a 16bit UTF-8-encoded character */
> - while (chars_sent <= count - 3) {
> + while (iov_iter_count(to)) {
> + u_char s[3];
> + int l, n;
> + u16 ch;
> +
> if (speakup_info.flushing) {
> speakup_info.flushing = 0;
> ch = '\x18';
> @@ -244,60 +244,47 @@ static ssize_t softsynthx_read(struct file *fp, char __user *buf, size_t count,
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
>
> if ((!unicode && ch < 0x100) || (unicode && ch < 0x80)) {
> - u_char c = ch;
> -
> - if (copy_to_user(cp, &c, 1))
> - return -EFAULT;
> -
> - chars_sent++;
> - cp++;
> + s[0] = ch;
> + l = 1;
> } else if (unicode && ch < 0x800) {
> - u_char s[2] = {
> - 0xc0 | (ch >> 6),
> - 0x80 | (ch & 0x3f)
> - };
> -
> - if (copy_to_user(cp, s, sizeof(s)))
> - return -EFAULT;
> -
> - chars_sent += sizeof(s);
> - cp += sizeof(s);
> + s[0] = 0xc0 | (ch >> 6);
> + s[1] = 0x80 | (ch & 0x3f);
> + l = 2;
> } else if (unicode) {
> - u_char s[3] = {
> - 0xe0 | (ch >> 12),
> - 0x80 | ((ch >> 6) & 0x3f),
> - 0x80 | (ch & 0x3f)
> - };
> -
> - if (copy_to_user(cp, s, sizeof(s)))
> - return -EFAULT;
> -
> - chars_sent += sizeof(s);
> - cp += sizeof(s);
> + s[0] = 0xe0 | (ch >> 12);
> + s[1] = 0x80 | ((ch >> 6) & 0x3f);
> + s[2] = 0x80 | (ch & 0x3f);
> + l = 3;
> }
> -
> + n = copy_to_iter(s, l, to);
> + if (n != l) {
> + iov_iter_revert(to, n);
> + spin_lock_irqsave(&speakup_info.spinlock, flags);
> + break;
> + }
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
