Sanity check TTY input and bail on problems

Also, introduce module parameter 'accept_flaky_uart'.

If your adapter or its connection is unreliable, set this option to
true to try and make the best of a bad situation, but undefined
behavior is prone to occur.

diff --git a/module/elmcan.c b/module/elmcan.c
index 1be5da8f222998130fdbc0ac157e09c185b6190d..82ffc86f581c479d90ec3e0c963ae1a6def5cd68 100644 (file)
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -22,6 +22,7 @@
 
 #include <linux/atomic.h>
 #include <linux/bitops.h>
+#include <linux/ctype.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/if_ether.h>
@@ -45,6 +46,16 @@ MODULE_DESCRIPTION("ELM327 based CAN interface");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Max Staudt <max-linux@enpas.org>");
 
+/* If this is enabled, we'll try to make the best of the situation
+ * even if we receive unexpected characters on the line.
+ * No guarantees.
+ * Handle with care, it's likely your hardware is unreliable!
+ */
+static bool accept_flaky_uart = false;
+module_param_named(accept_flaky_uart, accept_flaky_uart, bool, 0444);
+MODULE_PARM_DESC(accept_flaky_uart, "Don't bail at the first invalid character. Behavior undefined.");
+
+
 /* Line discipline ID number */
 #ifndef N_ELMCAN
 #define N_ELMCAN 29
@@ -193,7 +204,6 @@ static void elm327_kick_into_cmd_mode(struct elmcan *elm)
                elm327_send(elm, ELM327_MAGIC_STRING, 1);
 
                elm->state = ELM_GETMAGICCHAR;
-               elm->rxfill = 0;
        }
 }
 
@@ -361,6 +371,12 @@ static inline void elm327_hw_failure(struct elmcan *elm)
   * (assumes elm->lock taken)                                  *
   ************************************************************************/
 
+static bool elm327_is_ready_char(char c)
+{
+       return (c & 0x3f) == ELM327_READY_CHAR;
+}
+
+
 static void elm327_parse_error(struct elmcan *elm, int len)
 {
        struct can_frame frame;
@@ -442,6 +458,23 @@ static int elm327_parse_frame(struct elmcan *elm, int len)
                }
        }
 
+       /* If we accept stray characters coming in:
+        * Check for stray characters on a payload line.
+        * No idea what causes this.
+        */
+       if (accept_flaky_uart
+           && hexlen < len
+           && !isdigit(elm->rxbuf[hexlen])
+           && !isupper(elm->rxbuf[hexlen])
+           && '<' != elm->rxbuf[hexlen]
+           && ' ' != elm->rxbuf[hexlen]) {
+               /* The line is likely garbled anyway, so bail.
+                * The main code will restart listening.
+                */
+               elm327_kick_into_cmd_mode(elm);
+               return 3;
+       }
+
        /* Use spaces in CAN ID to distinguish 29 or 11 bit address length.
         * No out-of-bounds access:
         * We use the fact that we can always read from elm->rxbuf.
@@ -554,8 +587,8 @@ static void elm327_parse_line(struct elmcan *elm, int len)
                                /* Parse an error line. */
                                elm327_parse_error(elm, len);
 
-                               /* After the error line, we expect a prompt. */
-                               elm->state = ELM_GETPROMPT;
+                               /* Start afresh. */
+                               elm327_kick_into_cmd_mode(elm);
                        }
                        break;
                default:
@@ -673,7 +706,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
                                elm->state = ELM_GETPROMPT;
                                i++;
                                break;
-                       } else if ((elm->rxbuf[i] & 0x3f) == ELM327_READY_CHAR) {
+                       } else if (elm327_is_ready_char(elm->rxbuf[i])) {
                                /* Mask 0xc0 to work around hardware bugs */
                                elm327_send(elm, ELM327_MAGIC_STRING, 1);
                                i++;
@@ -691,7 +724,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
                 * Bits 0xc0 are sometimes set (randomly), hence the mask.
                 * Probably bad hardware.
                 */
-               if ((elm->rxbuf[elm->rxfill - 1] & 0x3f) == ELM327_READY_CHAR) {
+               if (elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) {
                        elm327_handle_prompt(elm);
                }
 
@@ -715,7 +748,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
                        return;
                } else if (len == elm->rxfill) {
                        if (elm->state == ELM_RECEIVING
-                               && elm->rxbuf[elm->rxfill - 1] == ELM327_READY_CHAR) {
+                               && elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) {
                                /* The ELM327's AT ST response timeout ran out,
                                 * so we got a prompt.
                                 * Clear RX buffer and restart listening.
@@ -965,9 +998,46 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
                        put_elm(elm);
                        return;
                }
+
+               /* Ignore NUL characters, which the PIC microcontroller may
+                * inadvertently insert due to a known hardware bug.
+                * See ELM327 documentation, which refers to a Microchip PIC
+                * bug description.
+                */
                if (*cp != 0) {
+                       /* Check for stray characters on the UART line.
+                        * No idea what causes this.
+                        */
+                       if (!accept_flaky_uart
+                           && !isdigit(*cp)
+                           && !isupper(*cp)
+                           && ELM327_MAGIC_CHAR != *cp
+                           && ELM327_READY_CHAR != *cp
+                           && '<' != *cp
+                           && 'a' != *cp
+                           && 'b' != *cp
+                           && 'v' != *cp
+                           && '.' != *cp
+                           && '?' != *cp
+                           && '\r' != *cp
+                           && ' ' != *cp) {
+                               /* We've received an invalid character, so bail.
+                                * There's something wrong with the ELM327, or
+                                * with the UART line.
+                                */
+                               netdev_err(elm->dev,
+                                       "Received illegal character %02x.\n",
+                                       *cp);
+                               elm327_hw_failure(elm);
+                               spin_unlock_bh(&elm->lock);
+
+                               put_elm(elm);
+                               return;
+                       }
+
                        elm->rxbuf[elm->rxfill++] = *cp;
                }
+
                cp++;
        }
 

diff --git a/readme.rst b/readme.rst
index d2aced218fb744c4dfd48895c2590bf73660b3fc..23920d828885a8505b59918753a3df7619957a03 100644 (file)
--- a/readme.rst
+++ b/readme.rst
@@ -92,6 +92,21 @@ sheet. This needs to be done before attaching the line discipline.
 
 
 
+Module parameters
+------------------
+
+- ``accept_flaky_uart`` - Try to live with a bad controller or UART line
+
+       Some adapters and/or their connection are unreliable. Enable this
+       option to try and work around the situation. This is a best-effort
+       workaround, so undefined behavior will likely occur sooner or later.
+
+       LOAD TIME::
+
+               module/kernel parameter: accept_flaky_uart=[0|1]
+
+
+
 Known limitations of the controller
 ------------------------------------