Handle tty->ops->write() returning error
[elmcan.git] / module / elmcan.c
index a62135e74250dd6aeb5dbb556788923b15405fc7..cde48caaf330062c3859828594ebc522ed62e55f 100644 (file)
@@ -20,7 +20,9 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 
+#include <linux/atomic.h>
 #include <linux/bitops.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/if_ether.h>
 #include <linux/kernel.h>
@@ -45,7 +47,7 @@ MODULE_AUTHOR("Max Staudt <max-linux@enpas.org>");
 
 /* Line discipline ID number */
 #ifndef N_ELMCAN
-#define N_ELMCAN 26
+#define N_ELMCAN 29
 #endif
 
 #define ELM327_CAN_CONFIG_SEND_SFF           0x8000
@@ -55,6 +57,7 @@ MODULE_AUTHOR("Max Staudt <max-linux@enpas.org>");
 
 #define ELM327_MAGIC_CHAR 'y'
 #define ELM327_MAGIC_STRING "y"
+#define ELM327_READY_CHAR '>'
 
 
 /* Bits in elm->cmds_todo */
@@ -83,6 +86,15 @@ struct elmcan {
        /* Per-channel lock */
        spinlock_t              lock;
 
+       /* Keep track of how many things are using this struct.
+        * Once it reaches 0, we are in the process of cleaning up,
+        * and new operations will be cancelled immediately.
+        * Use atomic_t rather than refcount_t because we deliberately
+        * decrement to 0, and refcount_dec() spills a WARN_ONCE in
+        * that case.
+        */
+       atomic_t                refcount;
+
        /* TTY TX helpers */
        struct work_struct      tx_work;        /* Flushes TTY TX buffer   */
        unsigned char           txbuf[32];
@@ -117,8 +129,15 @@ struct elmcan {
 };
 
 
-static DEFINE_SPINLOCK(elmcan_open_lock);
+/* A lock for all tty->disc_data handled by this ldisc.
+ * This is to prevent a case where tty->disc_data is set to NULL,
+ * yet someone is still trying to dereference it.
+ * Without this, we cannot do a clean shutdown.
+ */
+static DEFINE_SPINLOCK(elmcan_discdata_lock);
+
 
+static inline void elm327_panic(struct elmcan *elm);
 
 
 
@@ -144,6 +163,11 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
         */
        set_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
        actual = elm->tty->ops->write(elm->tty, elm->txbuf, len);
+       if (actual < 0) {
+               pr_err("Failed to write to tty for %s.\n", elm->dev->name);
+               elm327_panic(elm);
+       }
+
        elm->txleft = len - actual;
        elm->txhead = elm->txbuf + actual;
 }
@@ -607,7 +631,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
                                elm->state = ELM_GETPROMPT;
                                i++;
                                break;
-                       } else if (elm->rxbuf[i] == '>') {
+                       } else if (elm->rxbuf[i] == ELM327_READY_CHAR) {
                                elm327_send(elm, ELM327_MAGIC_STRING, 1);
                                i++;
                                break;
@@ -621,7 +645,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
 
        case ELM_GETPROMPT:
                /* Wait for '>' */
-               if (elm->rxbuf[elm->rxfill - 1] == '>') {
+               if (elm->rxbuf[elm->rxfill - 1] == ELM327_READY_CHAR) {
                        elm327_handle_prompt(elm);
                }
 
@@ -644,7 +668,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
                        elm327_panic(elm);
                } else if (len == elm->rxfill) {
                        if (elm->state == ELM_RECEIVING
-                               && elm->rxbuf[elm->rxfill - 1] == '>') {
+                               && elm->rxbuf[elm->rxfill - 1] == ELM327_READY_CHAR) {
                                /* The ELM327's AT ST response timeout ran out,
                                 * so we got a prompt.
                                 * Clear RX buffer and restart listening.
@@ -820,6 +844,44 @@ static const struct net_device_ops elmcan_netdev_ops = {
   * (takes elm->lock)                                          *
   ************************************************************************/
 
+/*
+ * Get a reference to our struct, taking into account locks/refcounts.
+ * This is to ensure ordering in case we are shutting down, and to ensure
+ * there is a refcount at all (because tty->disc_data may be NULL).
+ */
+static struct elmcan* get_elm(struct tty_struct *tty)
+{
+       struct elmcan *elm;
+       bool got_ref;
+
+       /* Lock all elmcan TTYs, so tty->disc_data can't become NULL
+        * the moment before we increase the reference counter.
+        */
+       spin_lock_bh(&elmcan_discdata_lock);
+       elm = (struct elmcan *) tty->disc_data;
+
+       if (!elm) {
+               spin_unlock_bh(&elmcan_discdata_lock);
+               return NULL;
+       }
+
+       got_ref = atomic_inc_not_zero(&elm->refcount);
+       spin_unlock_bh(&elmcan_discdata_lock);
+
+       if (!got_ref) {
+               return NULL;
+       }
+
+       return elm;
+}
+
+static void put_elm(struct elmcan *elm)
+{
+       atomic_dec(&elm->refcount);
+}
+
+
+
 /*
  * Handle the 'receiver data ready' interrupt.
  * This function is called by the 'tty_io' module in the kernel when
@@ -831,7 +893,7 @@ static const struct net_device_ops elmcan_netdev_ops = {
 static void elmcan_ldisc_rx(struct tty_struct *tty,
                        const unsigned char *cp, char *fp, int count)
 {
-       struct elmcan *elm = (struct elmcan *) tty->disc_data;
+       struct elmcan *elm = get_elm(tty);
 
        if (!elm)
                return;
@@ -844,6 +906,8 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
                        spin_lock_bh(&elm->lock);
                        elm327_panic(elm);
                        spin_unlock_bh(&elm->lock);
+
+                       put_elm(elm);
                        return;
                }
                if (*cp != 0) {
@@ -858,12 +922,16 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
                spin_lock_bh(&elm->lock);
                elm327_panic(elm);
                spin_unlock_bh(&elm->lock);
+
+               put_elm(elm);
                return;
        }
 
        spin_lock_bh(&elm->lock);
        elm327_parse_rxbuf(elm);
        spin_unlock_bh(&elm->lock);
+
+       put_elm(elm);
 }
 
 /*
@@ -872,6 +940,9 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
  */
 static void elmcan_ldisc_tx_worker(struct work_struct *work)
 {
+       /* No need to use get_elm() here, as we'll always flush workers
+        * befory destroying the elmcan object.
+        */
        struct elmcan *elm = container_of(work, struct elmcan, tx_work);
        ssize_t actual;
 
@@ -910,9 +981,14 @@ static void elmcan_ldisc_tx_worker(struct work_struct *work)
  */
 static void elmcan_ldisc_tx_wakeup(struct tty_struct *tty)
 {
-       struct elmcan *elm = tty->disc_data;
+       struct elmcan *elm = get_elm(tty);
+
+       if (!elm)
+               return;
 
        schedule_work(&elm->tx_work);
+
+       put_elm(elm);
 }
 
 
@@ -949,16 +1025,6 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
        if (!tty->ops->write)
                return -EOPNOTSUPP;
 
-       elm = tty->disc_data;
-
-       /* First make sure we're not already connected.
-        * Also, protect against simlutaneous open calls. */
-       spin_lock_bh(&elmcan_open_lock);
-       if (elm) {
-               spin_unlock_bh(&elmcan_open_lock);
-               return -EEXIST;
-       }
-       spin_unlock_bh(&elmcan_open_lock);
 
        /* OK.  Find a free elmcan channel to use. */
        dev = alloc_candev(sizeof(struct elmcan), 0);
@@ -970,6 +1036,7 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
        tty->receive_room = 65536; /* We don't flow control */
        elm->txleft = 0; /* Clear TTY TX buffer */
        spin_lock_init(&elm->lock);
+       atomic_set(&elm->refcount, 1);
        INIT_WORK(&elm->tx_work, elmcan_ldisc_tx_worker);
 
        /* Configure CAN metadata */
@@ -1010,15 +1077,32 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
  */
 static void elmcan_ldisc_close(struct tty_struct *tty)
 {
-       struct elmcan *elm = (struct elmcan *) tty->disc_data;
+       /* Use get_elm() to synchronize against other users */
+       struct elmcan *elm = get_elm(tty);
 
-       /* First make sure we're connected. */
        if (!elm)
                return;
 
-       /* Flush network side */
+       /* Tear down network side.
+        * unregister_netdev() calls .ndo_stop() so we don't have to.
+        */
        unregister_candev(elm->dev);
 
+       /* Decrease the refcount twice, once for our own get_elm(),
+        * and once to remove the count of 1 that we set in _open().
+        * Once it reaches 0, we can safely destroy it.
+        */
+       put_elm(elm);
+       put_elm(elm);
+
+       /* Spin until refcount reaches 0 */
+       while(atomic_read(&elm->refcount) > 0)
+               msleep(1);
+
+       /* At this point, all ldisc calls to us will be no-ops.
+        * Since the refcount is 0, they are bailing immediately.
+        */
+
        /* Mark channel as dead */
        spin_lock_bh(&elm->lock);
        tty->disc_data = NULL;
@@ -1028,6 +1112,8 @@ static void elmcan_ldisc_close(struct tty_struct *tty)
        /* Flush TTY side */
        flush_work(&elm->tx_work);
 
+       netdev_info(elm->dev, "elmcan off %s.\n", tty->name);
+
        /* Free our memory */
        free_candev(elm->dev);
 }
@@ -1042,7 +1128,7 @@ static int elmcan_ldisc_hangup(struct tty_struct *tty)
 static int elmcan_ldisc_ioctl(struct tty_struct *tty, struct file *file,
                        unsigned int cmd, unsigned long arg)
 {
-       struct elmcan *elm = (struct elmcan *) tty->disc_data;
+       struct elmcan *elm = get_elm(tty);
        unsigned int tmp;
 
        /* First make sure we're connected. */
@@ -1052,14 +1138,20 @@ static int elmcan_ldisc_ioctl(struct tty_struct *tty, struct file *file,
        switch (cmd) {
        case SIOCGIFNAME:
                tmp = strlen(elm->ifname) + 1;
-               if (copy_to_user((void __user *)arg, elm->ifname, tmp))
+               if (copy_to_user((void __user *)arg, elm->ifname, tmp)) {
+                       put_elm(elm);
                        return -EFAULT;
+               }
+
+               put_elm(elm);
                return 0;
 
        case SIOCSIFHWADDR:
+               put_elm(elm);
                return -EINVAL;
 
        default:
+               put_elm(elm);
                return tty_mode_ioctl(tty, file, cmd, arg);
        }
 }