linux-kernel - Re: [PATCH 4.4 02/15] [media] siano: make it work again with CONFIG_VMAP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170222182022.07d68e85@vento.lan>
Date:   Wed, 22 Feb 2017 18:20:22 -0300
From:   Mauro Carvalho Chehab <mchehab@...pensource.com>
To:     Eddie Chapman <eddie@...k.net>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 4.4 02/15] [media] siano: make it work again with
 CONFIG_VMAP_STACK

Em Wed, 22 Feb 2017 21:07:24 +0000
Eddie Chapman <eddie@...k.net> escreveu:

> On 21/02/17 13:01, Greg Kroah-Hartman wrote:
> > 4.4-stable review patch.  If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Mauro Carvalho Chehab <mchehab@...pensource.com>
> >
> > commit f9c85ee67164b37f9296eab3b754e543e4e96a1c upstream.
> >
> > Reported as a Kaffeine bug:
> > 	https://bugs.kde.org/show_bug.cgi?id=375811
> >
> > The USB control messages require DMA to work. We cannot pass
> > a stack-allocated buffer, as it is not warranted that the
> > stack would be into a DMA enabled area.
> >
> > On Kernel 4.9, the default is to not accept DMA on stack anymore
> > on x86 architecture. On other architectures, this has been a
> > requirement since Kernel 2.2. So, after this patch, this driver
> > should likely work fine on all archs.
> >
> > Tested with USB ID 2040:5510: Hauppauge Windham
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@...pensource.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> >
> > ---
> >  drivers/media/usb/siano/smsusb.c |   18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > --- a/drivers/media/usb/siano/smsusb.c
> > +++ b/drivers/media/usb/siano/smsusb.c
> > @@ -200,22 +200,30 @@ static int smsusb_start_streaming(struct
> >  static int smsusb_sendrequest(void *context, void *buffer, size_t size)
> >  {
> >  	struct smsusb_device_t *dev = (struct smsusb_device_t *) context;
> > -	struct sms_msg_hdr *phdr = (struct sms_msg_hdr *) buffer;
> > -	int dummy;
> > +	struct sms_msg_hdr *phdr;
> > +	int dummy, ret;
> >
> >  	if (dev->state != SMSUSB_ACTIVE) {
> >  		pr_debug("Device not active yet\n");
> >  		return -ENOENT;
> >  	}
> >
> > +	phdr = kmalloc(size, GFP_KERNEL);
> > +	if (!phdr)
> > +		return -ENOMEM;
> > +	memcpy(phdr, buffer, size);
> > +
> >  	pr_debug("sending %s(%d) size: %d\n",
> >  		  smscore_translate_msg(phdr->msg_type), phdr->msg_type,
> >  		  phdr->msg_length);
> >
> >  	smsendian_handle_tx_message((struct sms_msg_data *) phdr);
> > -	smsendian_handle_message_header((struct sms_msg_hdr *)buffer);
> > -	return usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 2),
> > -			    buffer, size, &dummy, 1000);
> > +	smsendian_handle_message_header((struct sms_msg_hdr *)phdr);
> > +	ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 2),
> > +			    phdr, size, &dummy, 1000);
> > +
> > +	kfree(phdr);
> > +	return ret;
> >  }
> >
> >  static char *smsusb1_fw_lkup[] = {  
> 
> Hello Greg,
> 
> According to the bug report linked to in the commit message this is only 
> needed in 4.9 and higher.

Actually, it makes this device work on ARM platforms too.

The thing is, although forbidden since Kernel 2.2, on x86, using
the stack used to work (on most cases). So, there are many drivers
that were written without taking care of not using the stack before
sending an URB.

So, the best is to backport this patch to -stable Kernels as well,
as it will avoid potential (rare) issues on x86, but it will also
make the driver to work on non-x86 archs.

I updated the patch description to reflect it, but maybe I was not
too clear ;)


Thanks,
Mauro