From f1ee89d5b007620353fb7dec1d34db4baa1a5571 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Sun, 10 Oct 2010 21:50:44 +0000 Subject: ATM: solos-pci, remove use after free Stanse found we do in console_show: kfree_skb(skb); return skb->len; which is not good. Fix that by remembering the len and use it in the function instead. Signed-off-by: Jiri Slaby Cc: Chas Williams Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- drivers/atm/solos-pci.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/atm') diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c index f916ddf63938..f46138ab38b6 100644 --- a/drivers/atm/solos-pci.c +++ b/drivers/atm/solos-pci.c @@ -444,6 +444,7 @@ static ssize_t console_show(struct device *dev, struct device_attribute *attr, struct atm_dev *atmdev = container_of(dev, struct atm_dev, class_dev); struct solos_card *card = atmdev->dev_data; struct sk_buff *skb; + unsigned int len; spin_lock(&card->cli_queue_lock); skb = skb_dequeue(&card->cli_queue[SOLOS_CHAN(atmdev)]); @@ -451,11 +452,12 @@ static ssize_t console_show(struct device *dev, struct device_attribute *attr, if(skb == NULL) return sprintf(buf, "No data.\n"); - memcpy(buf, skb->data, skb->len); - dev_dbg(&card->dev->dev, "len: %d\n", skb->len); + len = skb->len; + memcpy(buf, skb->data, len); + dev_dbg(&card->dev->dev, "len: %d\n", len); kfree_skb(skb); - return skb->len; + return len; } static int send_command(struct solos_card *card, int dev, const char *buf, size_t size) -- cgit v1.2.3 From ec622ab072e95cf96e78ab4d39d4eea874518c80 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Sun, 10 Oct 2010 23:26:57 +0000 Subject: ATM: iphase, remove sleep-inside-atomic Stanse found that ia_init_one locks a spinlock and inside of that it calls ia_start which calls: * request_irq * tx_init which does kmalloc(GFP_KERNEL) Both of them can thus sleep and result in a deadlock. I don't see a reason to have a per-device spinlock there which is used only there and inited right before the lock location. So remove it completely. Signed-off-by: Jiri Slaby Cc: Chas Williams Signed-off-by: David S. Miller --- drivers/atm/iphase.c | 6 ------ drivers/atm/iphase.h | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/atm') diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c index ee9ddeb53417..8cb0347dec28 100644 --- a/drivers/atm/iphase.c +++ b/drivers/atm/iphase.c @@ -3156,7 +3156,6 @@ static int __devinit ia_init_one(struct pci_dev *pdev, { struct atm_dev *dev; IADEV *iadev; - unsigned long flags; int ret; iadev = kzalloc(sizeof(*iadev), GFP_KERNEL); @@ -3188,19 +3187,14 @@ static int __devinit ia_init_one(struct pci_dev *pdev, ia_dev[iadev_count] = iadev; _ia_dev[iadev_count] = dev; iadev_count++; - spin_lock_init(&iadev->misc_lock); - /* First fixes first. I don't want to think about this now. */ - spin_lock_irqsave(&iadev->misc_lock, flags); if (ia_init(dev) || ia_start(dev)) { IF_INIT(printk("IA register failed!\n");) iadev_count--; ia_dev[iadev_count] = NULL; _ia_dev[iadev_count] = NULL; - spin_unlock_irqrestore(&iadev->misc_lock, flags); ret = -EINVAL; goto err_out_deregister_dev; } - spin_unlock_irqrestore(&iadev->misc_lock, flags); IF_EVENT(printk("iadev_count = %d\n", iadev_count);) iadev->next_board = ia_boards; diff --git a/drivers/atm/iphase.h b/drivers/atm/iphase.h index b2cd20f549cb..077735e0e04b 100644 --- a/drivers/atm/iphase.h +++ b/drivers/atm/iphase.h @@ -1022,7 +1022,7 @@ typedef struct iadev_t { struct dle_q rx_dle_q; struct free_desc_q *rx_free_desc_qhead; struct sk_buff_head rx_dma_q; - spinlock_t rx_lock, misc_lock; + spinlock_t rx_lock; struct atm_vcc **rx_open; /* list of all open VCs */ u16 num_rx_desc, rx_buf_sz, rxing; u32 rx_pkt_ram, rx_tmp_cnt; -- cgit v1.2.3