MAX1111: Fix Race condition causing NULL pointer exception
PATCH v2 1/1 MAX1111: Fix Race condition causing NULL pointer exception
spi_sync call uses its spi_message parameter to keep completion information, having this structure static is not thread-safe, potentially causing one thread having pointers to memory on or above other threads stack. use mutex to prevent multiple access
--- drivers/hwmon/max1111.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
drivers/hwmon/max1111.c +12 -0
@@ -40,6 +40,7 @@ struct max1111_data {struct spi_transfer xfer[2 ];uint8_t *tx_buf;uint8_t *rx_buf; +struct mutex msg_lock_mutex; };static int max1111_read (struct device *dev,int channel)
@@ -48,6 +49,11 @@ static int max1111_read(struct device *dev, int channel)uint8_t v1, v2;int err; +/* spi_sync requires data not to be freed before function returns +* for static data, any access is dangerous, use locks +*/ +mutex_lock (&data->msg_lock_mutex); + data->tx_buf[0 ] = (channel <<MAX1111_CTRL_SEL_SH ) |MAX1111_CTRL_PD0 |MAX1111_CTRL_PD1 |MAX1111_CTRL_SGL |MAX1111_CTRL_UNI |MAX1111_CTRL_STR ;
@@ -55,12 +61,15 @@ static int max1111_read(struct device *dev, int channel) err =spi_sync (data->spi, &data->msg);if (err <0 ) {dev_err (dev,"spi_sync failed with %d\n" , err); +mutex_unlock (&data->msg_lock_mutex);return err; } v1 = data->rx_buf[0 ]; v2 = data->rx_buf[1 ]; +mutex_unlock (&data->msg_lock_mutex); +if ((v1 &0xc0 ) || (v2 &0x3f ))return -EINVAL ;
@@ -176,6 +185,8 @@ static int __devinit max1111_probe(struct spi_device *spi)if (err) goto err_free_data; +mutex_init (&data->msg_lock_mutex); + data->spi = spi;spi_set_drvdata (spi, data);
@@ -213,6 +224,7 @@ static int __devexit max1111_remove(struct spi_device *spi)hwmon_device_unregister (data->hwmon_dev);sysfs_remove_group (&spi->dev.kobj, &max1111_attr_group); +mutex_destroy (data->msg_lock_mutex);kfree (data->rx_buf);kfree (data->tx_buf);kfree (data);
Signature · 1 line
1.7.5.rc3
PATCH v2 1/1 MAX1111: Fix Race condition causing NULL pointer exception
Quoted 11 lines · Pavel Herrmann
spi_synccall uses itsspi_messageparameter to keep completion information, having this structure static is not thread-safe, potentially causing one thread having pointers to memory on or above other threads stack. use mutex to prevent multiple accessSigned-off-by Pavel Herrmann <[email protected]>Acked-by Russell King <[email protected]>Acked-by Pavel Machek <[email protected]>Acked-by Marek Vasut <[email protected]>Acked-by Cyril Hrubis <[email protected]>
Quoted 4 lines
--- drivers/hwmon/max1111.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
<formletter>
This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read Documentation/stable_kernel_rules.txt for how to do this properly.
</formletter>
PATCH v2 1/1 MAX1111: Fix Race condition causing NULL pointer exception
In reply to Pavel Herrmannanyone picked this yet?
Quoted 15 lines · On Friday 03 of June 2011 22:00:03 Pavel Herrmann wrote:
spi_synccall uses itsspi_messageparameter to keep completion information, having this structure static is not thread-safe, potentially causing one thread having pointers to memory on or above other threads stack. use mutex to prevent multiple accessSigned-off-by Pavel Herrmann <[email protected]>Acked-by Russell King <[email protected]>Acked-by Pavel Machek <[email protected]>Acked-by Marek Vasut <[email protected]>Acked-by Cyril Hrubis <[email protected]>--- drivers/hwmon/max1111.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)