Commit 60362158 authored by Jesper Nilsson's avatar Jesper Nilsson

CRIS: gpio: don't call copy_to_user()/copy_from_user() while holding spinlocks

copy_to_user()/copy_from_user() must not be used with spinlocks held.
Move locks inside each case so we have better control of when the locks
are held.

Also, since we use spinlocks, we don't need to hold the BKL, so remove it.
Reported-by: default avatarKulikov Vasiliy <segooon@gmail.com>
Signed-off-by: default avatarJesper Nilsson <jesper.nilsson@axis.com>
parent 16bc0fe5
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
#include <linux/errno.h> #include <linux/errno.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/fs.h> #include <linux/fs.h>
#include <linux/smp_lock.h>
#include <linux/string.h> #include <linux/string.h>
#include <linux/poll.h> #include <linux/poll.h>
#include <linux/init.h> #include <linux/init.h>
...@@ -46,7 +45,7 @@ static char gpio_name[] = "etrax gpio"; ...@@ -46,7 +45,7 @@ static char gpio_name[] = "etrax gpio";
static wait_queue_head_t *gpio_wq; static wait_queue_head_t *gpio_wq;
#endif #endif
static int gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg); static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
static ssize_t gpio_write(struct file *file, const char __user *buf, static ssize_t gpio_write(struct file *file, const char __user *buf,
size_t count, loff_t *off); size_t count, loff_t *off);
static int gpio_open(struct inode *inode, struct file *filp); static int gpio_open(struct inode *inode, struct file *filp);
...@@ -323,7 +322,6 @@ gpio_open(struct inode *inode, struct file *filp) ...@@ -323,7 +322,6 @@ gpio_open(struct inode *inode, struct file *filp)
if (!priv) if (!priv)
return -ENOMEM; return -ENOMEM;
lock_kernel();
priv->minor = p; priv->minor = p;
/* initialize the io/alarm struct */ /* initialize the io/alarm struct */
...@@ -358,7 +356,6 @@ gpio_open(struct inode *inode, struct file *filp) ...@@ -358,7 +356,6 @@ gpio_open(struct inode *inode, struct file *filp)
alarmlist = priv; alarmlist = priv;
spin_unlock_irqrestore(&gpio_lock, flags); spin_unlock_irqrestore(&gpio_lock, flags);
unlock_kernel();
return 0; return 0;
} }
...@@ -503,8 +500,7 @@ unsigned long inline setget_output(struct gpio_private *priv, unsigned long arg) ...@@ -503,8 +500,7 @@ unsigned long inline setget_output(struct gpio_private *priv, unsigned long arg)
static int static int
gpio_leds_ioctl(unsigned int cmd, unsigned long arg); gpio_leds_ioctl(unsigned int cmd, unsigned long arg);
static int static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
{ {
unsigned long flags; unsigned long flags;
unsigned long val; unsigned long val;
...@@ -514,54 +510,65 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -514,54 +510,65 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
if (_IOC_TYPE(cmd) != ETRAXGPIO_IOCTYPE) if (_IOC_TYPE(cmd) != ETRAXGPIO_IOCTYPE)
return -EINVAL; return -EINVAL;
spin_lock_irqsave(&gpio_lock, flags);
switch (_IOC_NR(cmd)) { switch (_IOC_NR(cmd)) {
case IO_READBITS: /* Use IO_READ_INBITS and IO_READ_OUTBITS instead */ case IO_READBITS: /* Use IO_READ_INBITS and IO_READ_OUTBITS instead */
// read the port // read the port
spin_lock_irqsave(&gpio_lock, flags);
if (USE_PORTS(priv)) { if (USE_PORTS(priv)) {
ret = *priv->port; ret = *priv->port;
} else if (priv->minor == GPIO_MINOR_G) { } else if (priv->minor == GPIO_MINOR_G) {
ret = (*R_PORT_G_DATA) & 0x7FFFFFFF; ret = (*R_PORT_G_DATA) & 0x7FFFFFFF;
} }
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_SETBITS: case IO_SETBITS:
// set changeable bits with a 1 in arg // set changeable bits with a 1 in arg
spin_lock_irqsave(&gpio_lock, flags);
if (USE_PORTS(priv)) { if (USE_PORTS(priv)) {
*priv->port = *priv->shadow |= *priv->port = *priv->shadow |=
((unsigned char)arg & priv->changeable_bits); ((unsigned char)arg & priv->changeable_bits);
} else if (priv->minor == GPIO_MINOR_G) { } else if (priv->minor == GPIO_MINOR_G) {
*R_PORT_G_DATA = port_g_data_shadow |= (arg & dir_g_out_bits); *R_PORT_G_DATA = port_g_data_shadow |= (arg & dir_g_out_bits);
} }
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_CLRBITS: case IO_CLRBITS:
// clear changeable bits with a 1 in arg // clear changeable bits with a 1 in arg
spin_lock_irqsave(&gpio_lock, flags);
if (USE_PORTS(priv)) { if (USE_PORTS(priv)) {
*priv->port = *priv->shadow &= *priv->port = *priv->shadow &=
~((unsigned char)arg & priv->changeable_bits); ~((unsigned char)arg & priv->changeable_bits);
} else if (priv->minor == GPIO_MINOR_G) { } else if (priv->minor == GPIO_MINOR_G) {
*R_PORT_G_DATA = port_g_data_shadow &= ~((unsigned long)arg & dir_g_out_bits); *R_PORT_G_DATA = port_g_data_shadow &= ~((unsigned long)arg & dir_g_out_bits);
} }
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_HIGHALARM: case IO_HIGHALARM:
// set alarm when bits with 1 in arg go high // set alarm when bits with 1 in arg go high
spin_lock_irqsave(&gpio_lock, flags);
priv->highalarm |= arg; priv->highalarm |= arg;
gpio_some_alarms = 1; gpio_some_alarms = 1;
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_LOWALARM: case IO_LOWALARM:
// set alarm when bits with 1 in arg go low // set alarm when bits with 1 in arg go low
spin_lock_irqsave(&gpio_lock, flags);
priv->lowalarm |= arg; priv->lowalarm |= arg;
gpio_some_alarms = 1; gpio_some_alarms = 1;
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_CLRALARM: case IO_CLRALARM:
// clear alarm for bits with 1 in arg /* clear alarm for bits with 1 in arg */
spin_lock_irqsave(&gpio_lock, flags);
priv->highalarm &= ~arg; priv->highalarm &= ~arg;
priv->lowalarm &= ~arg; priv->lowalarm &= ~arg;
{ {
/* Must update gpio_some_alarms */ /* Must update gpio_some_alarms */
struct gpio_private *p = alarmlist; struct gpio_private *p = alarmlist;
int some_alarms; int some_alarms;
spin_lock_irq(&gpio_lock);
p = alarmlist; p = alarmlist;
some_alarms = 0; some_alarms = 0;
while (p) { while (p) {
...@@ -572,11 +579,12 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -572,11 +579,12 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
p = p->next; p = p->next;
} }
gpio_some_alarms = some_alarms; gpio_some_alarms = some_alarms;
spin_unlock_irq(&gpio_lock);
} }
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_READDIR: /* Use IO_SETGET_INPUT/OUTPUT instead! */ case IO_READDIR: /* Use IO_SETGET_INPUT/OUTPUT instead! */
/* Read direction 0=input 1=output */ /* Read direction 0=input 1=output */
spin_lock_irqsave(&gpio_lock, flags);
if (USE_PORTS(priv)) { if (USE_PORTS(priv)) {
ret = *priv->dir_shadow; ret = *priv->dir_shadow;
} else if (priv->minor == GPIO_MINOR_G) { } else if (priv->minor == GPIO_MINOR_G) {
...@@ -585,30 +593,40 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -585,30 +593,40 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
*/ */
ret = (dir_g_shadow | dir_g_out_bits) & 0x7FFFFFFF; ret = (dir_g_shadow | dir_g_out_bits) & 0x7FFFFFFF;
} }
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_SETINPUT: /* Use IO_SETGET_INPUT instead! */ case IO_SETINPUT: /* Use IO_SETGET_INPUT instead! */
/* Set direction 0=unchanged 1=input, /* Set direction 0=unchanged 1=input,
* return mask with 1=input * return mask with 1=input
*/ */
spin_lock_irqsave(&gpio_lock, flags);
ret = setget_input(priv, arg) & 0x7FFFFFFF; ret = setget_input(priv, arg) & 0x7FFFFFFF;
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_SETOUTPUT: /* Use IO_SETGET_OUTPUT instead! */ case IO_SETOUTPUT: /* Use IO_SETGET_OUTPUT instead! */
/* Set direction 0=unchanged 1=output, /* Set direction 0=unchanged 1=output,
* return mask with 1=output * return mask with 1=output
*/ */
spin_lock_irqsave(&gpio_lock, flags);
ret = setget_output(priv, arg) & 0x7FFFFFFF; ret = setget_output(priv, arg) & 0x7FFFFFFF;
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_SHUTDOWN: case IO_SHUTDOWN:
spin_lock_irqsave(&gpio_lock, flags);
SOFT_SHUTDOWN(); SOFT_SHUTDOWN();
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_GET_PWR_BT: case IO_GET_PWR_BT:
spin_lock_irqsave(&gpio_lock, flags);
#if defined (CONFIG_ETRAX_SOFT_SHUTDOWN) #if defined (CONFIG_ETRAX_SOFT_SHUTDOWN)
ret = (*R_PORT_G_DATA & ( 1 << CONFIG_ETRAX_POWERBUTTON_BIT)); ret = (*R_PORT_G_DATA & ( 1 << CONFIG_ETRAX_POWERBUTTON_BIT));
#else #else
ret = 0; ret = 0;
#endif #endif
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_CFG_WRITE_MODE: case IO_CFG_WRITE_MODE:
spin_lock_irqsave(&gpio_lock, flags);
priv->clk_mask = arg & 0xFF; priv->clk_mask = arg & 0xFF;
priv->data_mask = (arg >> 8) & 0xFF; priv->data_mask = (arg >> 8) & 0xFF;
priv->write_msb = (arg >> 16) & 0x01; priv->write_msb = (arg >> 16) & 0x01;
...@@ -624,28 +642,33 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -624,28 +642,33 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
priv->data_mask = 0; priv->data_mask = 0;
ret = -EPERM; ret = -EPERM;
} }
spin_unlock_irqrestore(&gpio_lock, flags);
break; break;
case IO_READ_INBITS: case IO_READ_INBITS:
/* *arg is result of reading the input pins */ /* *arg is result of reading the input pins */
spin_lock_irqsave(&gpio_lock, flags);
if (USE_PORTS(priv)) { if (USE_PORTS(priv)) {
val = *priv->port; val = *priv->port;
} else if (priv->minor == GPIO_MINOR_G) { } else if (priv->minor == GPIO_MINOR_G) {
val = *R_PORT_G_DATA; val = *R_PORT_G_DATA;
} }
spin_unlock_irqrestore(&gpio_lock, flags);
if (copy_to_user((void __user *)arg, &val, sizeof(val))) if (copy_to_user((void __user *)arg, &val, sizeof(val)))
ret = -EFAULT; ret = -EFAULT;
break; break;
case IO_READ_OUTBITS: case IO_READ_OUTBITS:
/* *arg is result of reading the output shadow */ /* *arg is result of reading the output shadow */
spin_lock_irqsave(&gpio_lock, flags);
if (USE_PORTS(priv)) { if (USE_PORTS(priv)) {
val = *priv->shadow; val = *priv->shadow;
} else if (priv->minor == GPIO_MINOR_G) { } else if (priv->minor == GPIO_MINOR_G) {
val = port_g_data_shadow; val = port_g_data_shadow;
} }
spin_unlock_irqrestore(&gpio_lock, flags);
if (copy_to_user((void __user *)arg, &val, sizeof(val))) if (copy_to_user((void __user *)arg, &val, sizeof(val)))
ret = -EFAULT; ret = -EFAULT;
break; break;
case IO_SETGET_INPUT: case IO_SETGET_INPUT:
/* bits set in *arg is set to input, /* bits set in *arg is set to input,
* *arg updated with current input pins. * *arg updated with current input pins.
*/ */
...@@ -654,7 +677,9 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -654,7 +677,9 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
ret = -EFAULT; ret = -EFAULT;
break; break;
} }
spin_lock_irqsave(&gpio_lock, flags);
val = setget_input(priv, val); val = setget_input(priv, val);
spin_unlock_irqrestore(&gpio_lock, flags);
if (copy_to_user((void __user *)arg, &val, sizeof(val))) if (copy_to_user((void __user *)arg, &val, sizeof(val)))
ret = -EFAULT; ret = -EFAULT;
break; break;
...@@ -666,33 +691,24 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -666,33 +691,24 @@ gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
ret = -EFAULT; ret = -EFAULT;
break; break;
} }
spin_lock_irqsave(&gpio_lock, flags);
val = setget_output(priv, val); val = setget_output(priv, val);
spin_unlock_irqrestore(&gpio_lock, flags);
if (copy_to_user((void __user *)arg, &val, sizeof(val))) if (copy_to_user((void __user *)arg, &val, sizeof(val)))
ret = -EFAULT; ret = -EFAULT;
break; break;
default: default:
spin_lock_irqsave(&gpio_lock, flags);
if (priv->minor == GPIO_MINOR_LEDS) if (priv->minor == GPIO_MINOR_LEDS)
ret = gpio_leds_ioctl(cmd, arg); ret = gpio_leds_ioctl(cmd, arg);
else else
ret = -EINVAL; ret = -EINVAL;
spin_unlock_irqrestore(&gpio_lock, flags);
} /* switch */ } /* switch */
spin_unlock_irqrestore(&gpio_lock, flags);
return ret; return ret;
} }
static int
gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
long ret;
lock_kernel();
ret = gpio_ioctl_unlocked(file, cmd, arg);
unlock_kernel();
return ret;
}
static int static int
gpio_leds_ioctl(unsigned int cmd, unsigned long arg) gpio_leds_ioctl(unsigned int cmd, unsigned long arg)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment