From 40d24c4d8a7430aa4dfd7a665fa3faf3b05b673f Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 15 Dec 2015 13:07:52 +0300 Subject: mISDN: fix a loop count There are two issue here. 1) cnt starts as maxloop + 1 so all these loops iterate one more time than intended. 2) At the end of the loop we test for "if (maxloop && !cnt)" but for the first two loops, we end with cnt equal to -1. Changing this to a pre-op means we end with cnt set to 0. Fixes: cae86d4a4e56 ('mISDN: Add driver for Infineon ISDN chipset family') Signed-off-by: Dan Carpenter Signed-off-by: David S. Miller --- drivers/isdn/hardware/mISDN/mISDNipac.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/hardware/mISDN/mISDNipac.c b/drivers/isdn/hardware/mISDN/mISDNipac.c index a77eea594b69..cb428b9ee441 100644 --- a/drivers/isdn/hardware/mISDN/mISDNipac.c +++ b/drivers/isdn/hardware/mISDN/mISDNipac.c @@ -1170,7 +1170,7 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) if (ipac->type & IPAC_TYPE_IPACX) { ista = ReadIPAC(ipac, ISACX_ISTA); - while (ista && cnt--) { + while (ista && --cnt) { pr_debug("%s: ISTA %02x\n", ipac->name, ista); if (ista & IPACX__ICA) ipac_irq(&ipac->hscx[0], ista); @@ -1182,7 +1182,7 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) } } else if (ipac->type & IPAC_TYPE_IPAC) { ista = ReadIPAC(ipac, IPAC_ISTA); - while (ista && cnt--) { + while (ista && --cnt) { pr_debug("%s: ISTA %02x\n", ipac->name, ista); if (ista & (IPAC__ICD | IPAC__EXD)) { istad = ReadISAC(isac, ISAC_ISTA); @@ -1200,7 +1200,7 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) ista = ReadIPAC(ipac, IPAC_ISTA); } } else if (ipac->type & IPAC_TYPE_HSCX) { - while (cnt) { + while (--cnt) { ista = ReadIPAC(ipac, IPAC_ISTAB + ipac->hscx[1].off); pr_debug("%s: B2 ISTA %02x\n", ipac->name, ista); if (ista) @@ -1211,7 +1211,6 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) mISDNisac_irq(isac, istad); if (0 == (ista | istad)) break; - cnt--; } } if (cnt > maxloop) /* only for ISAC/HSCX without PCI IRQ test */ -- cgit v1.2.3 From 74375c0528f724a4afa561d7bca0e2214ce5e3cf Mon Sep 17 00:00:00 2001 From: Tilman Schmidt Date: Tue, 15 Dec 2015 18:11:28 +0100 Subject: ser_gigaset: fix up NULL checks Commit f34d7a5b7010 ("tty: The big operations rework") changed tty->driver to tty->ops but left NULL checks for tty->driver untouched. Fix. Signed-off-by: Tilman Schmidt [pebolle: removed Fixes tag] Signed-off-by: Paul Bolle Signed-off-by: David S. Miller --- drivers/isdn/gigaset/ser-gigaset.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c index 375be509e95f..d8771b5d6904 100644 --- a/drivers/isdn/gigaset/ser-gigaset.c +++ b/drivers/isdn/gigaset/ser-gigaset.c @@ -67,7 +67,7 @@ static int write_modem(struct cardstate *cs) struct sk_buff *skb = bcs->tx_skb; int sent = -EOPNOTSUPP; - if (!tty || !tty->driver || !skb) + if (!tty || !tty->ops || !skb) return -EINVAL; if (!skb->len) { @@ -109,7 +109,7 @@ static int send_cb(struct cardstate *cs) unsigned long flags; int sent = 0; - if (!tty || !tty->driver) + if (!tty || !tty->ops) return -EFAULT; cb = cs->cmdbuf; @@ -432,7 +432,7 @@ static int gigaset_set_modem_ctrl(struct cardstate *cs, unsigned old_state, struct tty_struct *tty = cs->hw.ser->tty; unsigned int set, clear; - if (!tty || !tty->driver || !tty->ops->tiocmset) + if (!tty || !tty->ops || !tty->ops->tiocmset) return -EINVAL; set = new_state & ~old_state; clear = old_state & ~new_state; -- cgit v1.2.3 From ede03d306b8ba97c7988646aa503489ed421f05a Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Tue, 15 Dec 2015 18:11:29 +0100 Subject: ser_gigaset: turn nonsense checks into WARN_ON These checks do nothing useful to protect the code from races. On the other hand if the old code has been masking a real bug we would like to know about it. The check for tiocmset is kept because it is valid for a tty driver to have a NULL tiocmset method. That in itself is probably a mistake given modern coding practices - but needs fixing in the tty layer. Signed-off-by: Alan Cox Acked-by: Tilman Schmidt Signed-off-by: Paul Bolle Signed-off-by: David S. Miller --- drivers/isdn/gigaset/ser-gigaset.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c index d8771b5d6904..8e21f6afa832 100644 --- a/drivers/isdn/gigaset/ser-gigaset.c +++ b/drivers/isdn/gigaset/ser-gigaset.c @@ -67,8 +67,7 @@ static int write_modem(struct cardstate *cs) struct sk_buff *skb = bcs->tx_skb; int sent = -EOPNOTSUPP; - if (!tty || !tty->ops || !skb) - return -EINVAL; + WARN_ON(!tty || !tty->ops || !skb); if (!skb->len) { dev_kfree_skb_any(skb); @@ -109,8 +108,7 @@ static int send_cb(struct cardstate *cs) unsigned long flags; int sent = 0; - if (!tty || !tty->ops) - return -EFAULT; + WARN_ON(!tty || !tty->ops); cb = cs->cmdbuf; if (!cb) @@ -432,7 +430,9 @@ static int gigaset_set_modem_ctrl(struct cardstate *cs, unsigned old_state, struct tty_struct *tty = cs->hw.ser->tty; unsigned int set, clear; - if (!tty || !tty->ops || !tty->ops->tiocmset) + WARN_ON(!tty || !tty->ops); + /* tiocmset is an optional tty driver method */ + if (!tty->ops->tiocmset) return -EINVAL; set = new_state & ~old_state; clear = old_state & ~new_state; -- cgit v1.2.3 From 4c5e354a974214dfb44cd23fa0429327693bc3ea Mon Sep 17 00:00:00 2001 From: Tilman Schmidt Date: Tue, 15 Dec 2015 18:11:30 +0100 Subject: ser_gigaset: fix deallocation of platform device structure When shutting down the device, the struct ser_cardstate must not be kfree()d immediately after the call to platform_device_unregister() since the embedded struct platform_device is still in use. Move the kfree() call to the release method instead. Signed-off-by: Tilman Schmidt Fixes: 2869b23e4b95 ("drivers/isdn/gigaset: new M101 driver (v2)") Reported-by: Sasha Levin Signed-off-by: Paul Bolle Signed-off-by: David S. Miller --- drivers/isdn/gigaset/ser-gigaset.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c index 8e21f6afa832..635baaf34e95 100644 --- a/drivers/isdn/gigaset/ser-gigaset.c +++ b/drivers/isdn/gigaset/ser-gigaset.c @@ -368,19 +368,23 @@ static void gigaset_freecshw(struct cardstate *cs) tasklet_kill(&cs->write_tasklet); if (!cs->hw.ser) return; - dev_set_drvdata(&cs->hw.ser->dev.dev, NULL); platform_device_unregister(&cs->hw.ser->dev); - kfree(cs->hw.ser); - cs->hw.ser = NULL; } static void gigaset_device_release(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); + struct cardstate *cs = dev_get_drvdata(dev); /* adapted from platform_device_release() in drivers/base/platform.c */ kfree(dev->platform_data); kfree(pdev->resource); + + if (!cs) + return; + dev_set_drvdata(dev, NULL); + kfree(cs->hw.ser); + cs->hw.ser = NULL; } /* -- cgit v1.2.3 From 8aeb3c3d655e22d3aa5ba49f313157bd27354bb4 Mon Sep 17 00:00:00 2001 From: Tilman Schmidt Date: Tue, 15 Dec 2015 18:11:31 +0100 Subject: ser_gigaset: remove unnecessary kfree() calls from release method device->platform_data and platform_device->resource are never used and remain NULL through their entire life. Drops the kfree() calls for them from the device release method. Signed-off-by: Tilman Schmidt Signed-off-by: Paul Bolle Signed-off-by: David S. Miller --- drivers/isdn/gigaset/ser-gigaset.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c index 635baaf34e95..2a506fe0c8a4 100644 --- a/drivers/isdn/gigaset/ser-gigaset.c +++ b/drivers/isdn/gigaset/ser-gigaset.c @@ -373,13 +373,8 @@ static void gigaset_freecshw(struct cardstate *cs) static void gigaset_device_release(struct device *dev) { - struct platform_device *pdev = to_platform_device(dev); struct cardstate *cs = dev_get_drvdata(dev); - /* adapted from platform_device_release() in drivers/base/platform.c */ - kfree(dev->platform_data); - kfree(pdev->resource); - if (!cs) return; dev_set_drvdata(dev, NULL); -- cgit v1.2.3