[PATCH] parport: buffer overflow fix

Fix potential buffer overflow in case the device ID did not end in semicolon.
Also might fail to negotiate back to IEEE1284_MODE_COMPAT in case of failure.
parport_device_id did not return what Documentation/parport-lowlevel.txt said,
so I changed it to match it.

Determining device ID length is overly complicated, but Tim Waugh recalled on
linux-parport seeing some buggy device that might need it.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

1 files changed, 130 insertions, 63 deletions

diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c
index 4b48b31ec235..5c29e8222211 100644
--- a/drivers/parport/probe.c
+++ b/drivers/parport/probe.c
@@ -128,8 +128,131 @@ static void parse_data(struct parport *port, int device, char *str)
         kfree(txt);
 }
 
+/* Read up to count-1 bytes of device id. Terminate buffer with
+ * '\0'. Buffer begins with two Device ID length bytes as given by
+ * device. */
+static ssize_t parport_read_device_id (struct parport *port, char *buffer,
+                                       size_t count)
+{
+        unsigned char length[2];
+        unsigned lelen, belen;
+        size_t idlens[4];
+        unsigned numidlens;
+        unsigned current_idlen;
+        ssize_t retval;
+        size_t len;
+
+        /* First two bytes are MSB,LSB of inclusive length. */
+        retval = parport_read (port, length, 2);
+
+        if (retval < 0)
+                return retval;
+        if (retval != 2)
+                return -EIO;
+
+        if (count < 2)
+                return 0;
+        memcpy(buffer, length, 2);
+        len = 2;
+
+        /* Some devices wrongly send LE length, and some send it two
+         * bytes short. Construct a sorted array of lengths to try. */
+        belen = (length[0] << 8) + length[1];
+        lelen = (length[1] << 8) + length[0];
+        idlens[0] = min(belen, lelen);
+        idlens[1] = idlens[0]+2;
+        if (belen != lelen) {
+                int off = 2;
+                /* Don't try lenghts of 0x100 and 0x200 as 1 and 2 */
+                if (idlens[0] <= 2)
+                        off = 0;
+                idlens[off] = max(belen, lelen);
+                idlens[off+1] = idlens[off]+2;
+                numidlens = off+2;
+        }
+        else {
+                /* Some devices don't truly implement Device ID, but
+                 * just return constant nibble forever. This catches
+                 * also those cases. */
+                if (idlens[0] == 0 || idlens[0] > 0xFFF) {
+                        printk (KERN_DEBUG "%s: reported broken Device ID"
+                                " length of %#zX bytes\n",
+                                port->name, idlens[0]);
+                        return -EIO;
+                }
+                numidlens = 2;
+        }
+
+        /* Try to respect the given ID length despite all the bugs in
+         * the ID length. Read according to shortest possible ID
+         * first. */
+        for (current_idlen = 0; current_idlen < numidlens; ++current_idlen) {
+                size_t idlen = idlens[current_idlen];
+                if (idlen+1 >= count)
+                        break;
+
+                retval = parport_read (port, buffer+len, idlen-len);
+
+                if (retval < 0)
+                        return retval;
+                len += retval;
+
+                if (port->physport->ieee1284.phase != IEEE1284_PH_HBUSY_DAVAIL) {
+                        if (belen != len) {
+                                printk (KERN_DEBUG "%s: Device ID was %d bytes"
+                                        " while device told it would be %d"
+                                        " bytes\n",
+                                        port->name, len, belen);
+                        }
+                        goto done;
+                }
+
+                /* This might end reading the Device ID too
+                 * soon. Hopefully the needed fields were already in
+                 * the first 256 bytes or so that we must have read so
+                 * far. */
+                if (buffer[len-1] == ';') {
+                        printk (KERN_DEBUG "%s: Device ID reading stopped"
+                                " before device told data not available. "
+                                "Current idlen %d of %d, len bytes %02X %02X\n",
+                                port->name, current_idlen, numidlens,
+                                length[0], length[1]);
+                        goto done;
+                }
+        }
+        if (current_idlen < numidlens) {
+                /* Buffer not large enough, read to end of buffer. */
+                size_t idlen, len2;
+                if (len+1 < count) {
+                        retval = parport_read (port, buffer+len, count-len-1);
+                        if (retval < 0)
+                                return retval;
+                        len += retval;
+                }
+                /* Read the whole ID since some devices would not
+                 * otherwise give back the Device ID from beginning
+                 * next time when asked. */
+                idlen = idlens[current_idlen];
+                len2 = len;
+                while(len2 < idlen && retval > 0) {
+                        char tmp[4];
+                        retval = parport_read (port, tmp,
+                                               min(sizeof tmp, idlen-len2));
+                        if (retval < 0)
+                                return retval;
+                        len2 += retval;
+                }
+        }
+        /* In addition, there are broken devices out there that don't
+           even finish off with a semi-colon. We do not need to care
+           about those at this time. */
+ done:
+        buffer[len] = '\0';
+        return len;
+}
+
 /* Get Std 1284 Device ID. */
-ssize_t parport_device_id (int devnum, char *buffer, size_t len)
+ssize_t parport_device_id (int devnum, char *buffer, size_t count)
 {
         ssize_t retval = -ENXIO;
         struct pardevice *dev = parport_open (devnum, "Device ID probe",
@@ -139,76 +262,20 @@ ssize_t parport_device_id (int devnum, char *buffer, size_t len)
 
         parport_claim_or_block (dev);
 
-        /* Negotiate to compatibility mode, and then to device ID mode.
-         * (This is in case we are already in device ID mode.) */
+        /* Negotiate to compatibility mode, and then to device ID
+         * mode. (This so that we start form beginning of device ID if
+         * already in device ID mode.) */
         parport_negotiate (dev->port, IEEE1284_MODE_COMPAT);
         retval = parport_negotiate (dev->port,
                                     IEEE1284_MODE_NIBBLE | IEEE1284_DEVICEID);
 
         if (!retval) {
-                int idlen;
-                unsigned char length[2];
-
-                /* First two bytes are MSB,LSB of inclusive length. */
-                retval = parport_read (dev->port, length, 2);
-
-                if (retval != 2) goto end_id;
-
-                idlen = (length[0] << 8) + length[1] - 2;
-                /*
-                 * Check if the caller-allocated buffer is large enough
-                 * otherwise bail out or there will be an at least off by one.
-                 */
-                if (idlen + 1 < len)
-                        len = idlen;
-                else {
-                        retval = -EINVAL;
-                        goto out;
-                }
-                retval = parport_read (dev->port, buffer, len);
-
-                if (retval != len)
-                        printk (KERN_DEBUG "%s: only read %Zd of %Zd ID bytes\n",
-                                dev->port->name, retval,
-                                len);
-
-                /* Some printer manufacturers mistakenly believe that
-                   the length field is supposed to be _exclusive_.
-                   In addition, there are broken devices out there
-                   that don't even finish off with a semi-colon. */
-                if (buffer[len - 1] != ';') {
-                        ssize_t diff;
-                        diff = parport_read (dev->port, buffer + len, 2);
-                        retval += diff;
-
-                        if (diff)
-                                printk (KERN_DEBUG
-                                        "%s: device reported incorrect "
-                                        "length field (%d, should be %Zd)\n",
-                                        dev->port->name, idlen, retval);
-                        else {
-                                /* One semi-colon short of a device ID. */
-                                buffer[len++] = ';';
-                                printk (KERN_DEBUG "%s: faking semi-colon\n",
-                                        dev->port->name);
-
-                                /* If we get here, I don't think we
-                                   need to worry about the possible
-                                   standard violation of having read
-                                   more than we were told to.  The
-                                   device is non-compliant anyhow. */
-                        }
-                }
-
-        end_id:
-                buffer[len] = '\0';
+                retval = parport_read_device_id (dev->port, buffer, count);
                 parport_negotiate (dev->port, IEEE1284_MODE_COMPAT);
+                if (retval > 2)
+                        parse_data (dev->port, dev->daisy, buffer+2);
         }
 
-        if (retval > 2)
-                parse_data (dev->port, dev->daisy, buffer);
-
-out:
         parport_release (dev);
         parport_close (dev);
         return retval;