From a9dd7fe28742c6b22eb8f214a04c4d2bcb2c0899 Mon Sep 17 00:00:00 2001 From: Mark Asselstine Date: Wed, 14 May 2008 23:25:33 -0700 Subject: hysdn: Remove cli()/sti() calls. The use of cli()/sti() within the do/while was a way to ensure interrupts were only disabled for short periods of time while the bulk of the time interrupts were free to occur. The use of the spin lock has eliminated the need to play with interrupts in this way while still allowing for IO to be protected. The remaining 3 sti() calls seem unneeded now that at no other point in the driver is there a call to cli(). Signed-off-by: Mark Asselstine Acked-by: Karsten Keil Signed-off-by: Andrew Morton Signed-off-by: David S. Miller --- drivers/isdn/hysdn/boardergo.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/hysdn/boardergo.c b/drivers/isdn/hysdn/boardergo.c index 6cdbad3a992..3eb096f0ae1 100644 --- a/drivers/isdn/hysdn/boardergo.c +++ b/drivers/isdn/hysdn/boardergo.c @@ -64,10 +64,11 @@ ergo_interrupt(int intno, void *dev_id) } /* ergo_interrupt */ /******************************************************************************/ -/* ergo_irq_bh is the function called by the immediate kernel task list after */ -/* being activated with queue_task and no interrupts active. This task is the */ -/* only one handling data transfer from or to the card after booting. The task */ -/* may be queued from everywhere (interrupts included). */ +/* ergo_irq_bh will be called as part of the kernel clearing its shared work */ +/* queue sometime after a call to schedule_work has been made passing our */ +/* work_struct. This task is the only one handling data transfer from or to */ +/* the card after booting. The task may be queued from everywhere */ +/* (interrupts included). */ /******************************************************************************/ static void ergo_irq_bh(struct work_struct *ugli_api) @@ -90,7 +91,6 @@ ergo_irq_bh(struct work_struct *ugli_api) card->hw_lock = 1; /* we now lock the hardware */ do { - sti(); /* reenable other ints */ again = 0; /* assume loop not to be repeated */ if (!dpr->ToHyFlag) { @@ -110,7 +110,6 @@ ergo_irq_bh(struct work_struct *ugli_api) again = 1; /* restart loop */ } } /* a message has arrived for us */ - cli(); /* no further ints */ if (again) { dpr->ToHyInt = 1; dpr->ToPcInt = 1; /* interrupt to E1 for all cards */ @@ -242,7 +241,6 @@ ergo_writebootimg(struct HYSDN_CARD *card, unsigned char *buf, byteout(card->iobase + PCI9050_USER_IO, PCI9050_E1_RUN); /* start E1 processor */ /* the interrupts are still masked */ - sti(); msleep_interruptible(20); /* Timeout 20ms */ if (((tDpramBootSpooler *) card->dpram)->Len != DPRAM_SPOOLER_DATA_SIZE) { @@ -276,7 +274,6 @@ ergo_writebootseq(struct HYSDN_CARD *card, unsigned char *buf, int len) dst = sp->Data; /* point to data in spool structure */ buflen = sp->Len; /* maximum len of spooled data */ wr_mirror = sp->WrPtr; /* only once read */ - sti(); /* try until all bytes written or error */ i = 0x1000; /* timeout value */ @@ -380,7 +377,6 @@ ergo_waitpofready(struct HYSDN_CARD *card) #endif /* CONFIG_HYSDN_CAPI */ return (0); /* success */ } /* data has arrived */ - sti(); msleep_interruptible(50); /* Timeout 50ms */ } /* wait until timeout */ -- cgit v1.2.3-70-g09d2 From 01bbf2c7ddc93479eecebf8495848c0f362130c5 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Wed, 14 May 2008 23:27:18 -0700 Subject: hysdn: No longer broken on SMP. With the cli/sti code sorted out we think this driver is OK for use on SMP systems. Acked-by: Mark Asselstine Acked-by: Karsten Keil Signed-off-by: Andrew Morton Signed-off-by: David S. Miller --- drivers/isdn/hysdn/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/hysdn/Kconfig b/drivers/isdn/hysdn/Kconfig index c6d8a704298..c9e4231968e 100644 --- a/drivers/isdn/hysdn/Kconfig +++ b/drivers/isdn/hysdn/Kconfig @@ -3,7 +3,7 @@ # config HYSDN tristate "Hypercope HYSDN cards (Champ, Ergo, Metro) support (module only)" - depends on m && PROC_FS && PCI && BROKEN_ON_SMP + depends on m && PROC_FS && PCI help Say Y here if you have one of Hypercope's active PCI ISDN cards Champ, Ergo and Metro. You will then get a module called hysdn. -- cgit v1.2.3-70-g09d2 From 066b2118976e6e7cc50eed39e2747c75343a23c4 Mon Sep 17 00:00:00 2001 From: Marcin Slusarz Date: Wed, 14 May 2008 23:30:06 -0700 Subject: isdn/capi: Return proper errnos on module init. cdebug_init() is called from kcapi_init() which is module initialization function, so it must return negative values on errors. Signed-off-by: Marcin Slusarz Acked-by: Karsten Keil Signed-off-by: Andrew Morton Signed-off-by: David S. Miller --- drivers/isdn/capi/capiutil.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c index ebef4ce1b00..29419a8d31d 100644 --- a/drivers/isdn/capi/capiutil.c +++ b/drivers/isdn/capi/capiutil.c @@ -948,17 +948,17 @@ int __init cdebug_init(void) { g_cmsg= kmalloc(sizeof(_cmsg), GFP_KERNEL); if (!g_cmsg) - return ENOMEM; + return -ENOMEM; g_debbuf = kmalloc(sizeof(_cdebbuf), GFP_KERNEL); if (!g_debbuf) { kfree(g_cmsg); - return ENOMEM; + return -ENOMEM; } g_debbuf->buf = kmalloc(CDEBUG_GSIZE, GFP_KERNEL); if (!g_debbuf->buf) { kfree(g_cmsg); kfree(g_debbuf); - return ENOMEM;; + return -ENOMEM;; } g_debbuf->size = CDEBUG_GSIZE; g_debbuf->buf[0] = 0; -- cgit v1.2.3-70-g09d2 From 94b5e0ac694baba20efbe7d8ce6ff9cbe1776162 Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Thu, 22 May 2008 15:45:07 -0700 Subject: isdn: fix integer as NULL pointer warning drivers/isdn/hysdn/hycapi.c:465:42: warning: Using plain integer as NULL pointer drivers/isdn/hysdn/hycapi.c:467:44: warning: Using plain integer as NULL pointer drivers/isdn/hysdn/hycapi.c:469:42: warning: Using plain integer as NULL pointer Signed-off-by: Harvey Harrison Signed-off-by: Linus Torvalds --- drivers/isdn/hysdn/hycapi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/hysdn/hycapi.c b/drivers/isdn/hysdn/hycapi.c index d3999a8e9f8..53f6ad1235d 100644 --- a/drivers/isdn/hysdn/hycapi.c +++ b/drivers/isdn/hysdn/hycapi.c @@ -462,11 +462,11 @@ static int hycapi_read_proc(char *page, char **start, off_t off, default: s = "???"; break; } len += sprintf(page+len, "%-16s %s\n", "type", s); - if ((s = cinfo->version[VER_DRIVER]) != 0) + if ((s = cinfo->version[VER_DRIVER]) != NULL) len += sprintf(page+len, "%-16s %s\n", "ver_driver", s); - if ((s = cinfo->version[VER_CARDTYPE]) != 0) + if ((s = cinfo->version[VER_CARDTYPE]) != NULL) len += sprintf(page+len, "%-16s %s\n", "ver_cardtype", s); - if ((s = cinfo->version[VER_SERIAL]) != 0) + if ((s = cinfo->version[VER_SERIAL]) != NULL) len += sprintf(page+len, "%-16s %s\n", "ver_serial", s); len += sprintf(page+len, "%-16s %s\n", "cardname", cinfo->cardname); -- cgit v1.2.3-70-g09d2 From 495b36b15e17fb08434e3800959434f06a1a6fbc Mon Sep 17 00:00:00 2001 From: Alexey Dobriyan Date: Tue, 10 Jun 2008 12:49:31 -0700 Subject: isdn divas: fix proc creation 1. creating proc entry and not saving pointer to PDE and checking it is not going to work. 2. if proc entry wasn't created, no reason to remove it on error path. Signed-off-by: Alexey Dobriyan Signed-off-by: Andrew Morton Signed-off-by: David S. Miller --- drivers/isdn/hardware/eicon/divasmain.c | 1 - drivers/isdn/hardware/eicon/divasproc.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/hardware/eicon/divasmain.c b/drivers/isdn/hardware/eicon/divasmain.c index 5fcbdccd7a5..16a874bb156 100644 --- a/drivers/isdn/hardware/eicon/divasmain.c +++ b/drivers/isdn/hardware/eicon/divasmain.c @@ -806,7 +806,6 @@ static int DIVA_INIT_FUNCTION divas_init(void) if (!create_divas_proc()) { #ifdef MODULE - remove_divas_proc(); divas_unregister_chrdev(); divasfunc_exit(); #endif diff --git a/drivers/isdn/hardware/eicon/divasproc.c b/drivers/isdn/hardware/eicon/divasproc.c index fae895828a1..040827288ec 100644 --- a/drivers/isdn/hardware/eicon/divasproc.c +++ b/drivers/isdn/hardware/eicon/divasproc.c @@ -125,8 +125,8 @@ static const struct file_operations divas_fops = { int create_divas_proc(void) { - proc_create(divas_proc_name, S_IFREG | S_IRUGO, proc_net_eicon, - &divas_fops); + divas_proc_entry = proc_create(divas_proc_name, S_IFREG | S_IRUGO, + proc_net_eicon, &divas_fops); if (!divas_proc_entry) return (0); -- cgit v1.2.3-70-g09d2 From ea23ec26727b4df97b4965715f0519b6ddc0aa4b Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Tue, 10 Jun 2008 12:50:14 -0700 Subject: isdn: use simple_read_from_buffer() Signed-off-by: Akinobu Mita Acked-by: Karsten Keil Signed-off-by: Andrew Morton Signed-off-by: David S. Miller --- drivers/isdn/hysdn/hysdn_procconf.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/hysdn/hysdn_procconf.c b/drivers/isdn/hysdn/hysdn_procconf.c index 15906d005b0..484299b031f 100644 --- a/drivers/isdn/hysdn/hysdn_procconf.c +++ b/drivers/isdn/hysdn/hysdn_procconf.c @@ -207,30 +207,17 @@ hysdn_conf_write(struct file *file, const char __user *buf, size_t count, loff_t /* read conf file -> output card info data */ /*******************************************/ static ssize_t -hysdn_conf_read(struct file *file, char __user *buf, size_t count, loff_t * off) +hysdn_conf_read(struct file *file, char __user *buf, size_t count, loff_t *off) { char *cp; - int i; - if (file->f_mode & FMODE_READ) { - if (!(cp = file->private_data)) - return (-EFAULT); /* should never happen */ - i = strlen(cp); /* get total string length */ - if (*off < i) { - /* still bytes to transfer */ - cp += *off; /* point to desired data offset */ - i -= *off; /* remaining length */ - if (i > count) - i = count; /* limit length to transfer */ - if (copy_to_user(buf, cp, i)) - return (-EFAULT); /* copy error */ - *off += i; /* adjust offset */ - } else - return (0); - } else - return (-EPERM); /* no permission to read */ - - return (i); + if (!(file->f_mode & FMODE_READ)) + return -EPERM; /* no permission to read */ + + if (!(cp = file->private_data)) + return -EFAULT; /* should never happen */ + + return simple_read_from_buffer(buf, count, off, cp, strlen(cp)); } /* hysdn_conf_read */ /******************/ -- cgit v1.2.3-70-g09d2 From 093a44e71aa29157fb1611b00507d67c954099d6 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Thu, 12 Jun 2008 15:21:43 -0700 Subject: drivers/isdn/sc/ioctl.c: add missing kfree spid has been allocated in this function and so should be freed before leaving it, as in the other error handling cases. The semantic match that finds the problem is as follows: (http://www.emn.fr/x-info/coccinelle/) @r exists@ expression E,E1; statement S; position p1,p2,p3; @@ E =@p1 \(kmalloc\|kcalloc\|kzalloc\)(...) ... when != E = E1 if (E == NULL || ...) S ... when != E = E1 if@p2 (...) { ... when != kfree(E) } ... when != E = E1 kfree@p3(E); @forall@ position r.p2; expression r.E; int E1 != 0; @@ * if@p2 (...) { ... when != kfree(E) when strict return E1; } Signed-off-by: Julia Lawall Cc: Karsten Keil Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/isdn/sc/ioctl.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/isdn') diff --git a/drivers/isdn/sc/ioctl.c b/drivers/isdn/sc/ioctl.c index 7817d224492..1081091bbfa 100644 --- a/drivers/isdn/sc/ioctl.c +++ b/drivers/isdn/sc/ioctl.c @@ -226,6 +226,7 @@ int sc_ioctl(int card, scs_ioctl *data) */ if (copy_from_user(spid, data->dataptr, SCIOC_SPIDSIZE)) { kfree(rcvmsg); + kfree(spid); return -EFAULT; } -- cgit v1.2.3-70-g09d2 From 4fc89e3911aa5357b55b85b60c4beaeb8a48a290 Mon Sep 17 00:00:00 2001 From: Darren Jenkins Date: Sat, 12 Jul 2008 13:47:50 -0700 Subject: drivers/isdn/i4l/isdn_common.c fix small resource leak Coverity CID: 1356 RESOURCE_LEAK I found a very old patch for this that was Acked but did not get applied https://lists.linux-foundation.org/pipermail/kernel-janitors/2006-September/016362.html There looks to be a small leak in isdn_writebuf_stub() in isdn_common.c, when copy_from_user() returns an un-copied data length (length != 0). The below patch should be a minimally invasive fix. Signed-off-by: Darren Jenkins Acked-by: Karsten Keil Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/isdn/i4l/isdn_common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c index 0f3c66de69b..8d8c6b73616 100644 --- a/drivers/isdn/i4l/isdn_common.c +++ b/drivers/isdn/i4l/isdn_common.c @@ -1977,8 +1977,10 @@ isdn_writebuf_stub(int drvidx, int chan, const u_char __user * buf, int len) if (!skb) return -ENOMEM; skb_reserve(skb, hl); - if (copy_from_user(skb_put(skb, len), buf, len)) + if (copy_from_user(skb_put(skb, len), buf, len)) { + dev_kfree_skb(skb); return -EFAULT; + } ret = dev->drv[drvidx]->interface->writebuf_skb(drvidx, chan, 1, skb); if (ret <= 0) dev_kfree_skb(skb); -- cgit v1.2.3-70-g09d2