greybus: uart-gb: improve minor device number error checking

When alloc_minor() finds an available minor device number it
does not constrain the highest number desired.  Instead, it
relies on its caller, tty_gb_probe() to see if the returned
number indicates all minor numbers have been exhausted.

There are a couple problems with this--or rather with this
code.

First, if an allocation is attempted *after* GB_NUM_MINORS
is returned, a new number greater than (but not equal to)
GB_NUM_MINORS will be allocated, and that won't produce
any error condition.

Second, alloc_minor() can return an error code (like -ENOMEM).  And
its caller is only checking for GB_NUM_MINORS.  If an error code
is returned, tty_gb_probe() simply uses it.

Change alloc_minor() so it requests minor device numbers in the
range 0..(GB_NUM_MINORS-1), and use an error return to detect
when the minor device numbers have been exhausted.

If alloc_minor() returns -ENOSPC (from idr_alloc()), translate that
to -ENODEV.  The only other error we might see is -ENOMEM, and if
we get that, return it.

Finally, zero gb_tty->minor when it's released.  (If this is
actually important a reserved value like GB_NUM_MINORS should
be used instead to signify a gb_tty with no minor assigned.)

Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/greybus/uart-gb.c b/drivers/staging/greybus/uart-gb.c
index 13a23cde71fbaf939ab7c9fdd7c1170f9e199136..0e2507cdcd5ab8c4b4d19b4c12c632f0de77e5fe 100644 (file)
--- a/drivers/staging/greybus/uart-gb.c
+++ b/drivers/staging/greybus/uart-gb.c
@@ -80,19 +80,20 @@ static int alloc_minor(struct gb_tty *gb_tty)
        int minor;
 
        mutex_lock(&table_lock);
-       minor = idr_alloc(&tty_minors, gb_tty, 0, 0, GFP_KERNEL);
-       if (minor < 0)
-               goto error;
-       gb_tty->minor = minor;
-error:
+       minor = idr_alloc(&tty_minors, gb_tty, 0, GB_NUM_MINORS, GFP_KERNEL);
        mutex_unlock(&table_lock);
+       if (minor >= 0)
+               gb_tty->minor = minor;
        return minor;
 }
 
 static void release_minor(struct gb_tty *gb_tty)
 {
+       int minor = gb_tty->minor;
+
+       gb_tty->minor = 0;      /* Maybe should use an invalid value instead */
        mutex_lock(&table_lock);
-       idr_remove(&tty_minors, gb_tty->minor);
+       idr_remove(&tty_minors, minor);
        mutex_unlock(&table_lock);
 }
 
@@ -400,9 +401,12 @@ static int tty_gb_probe(struct greybus_device *gdev, const struct greybus_device
                return -ENOMEM;
 
        minor = alloc_minor(gb_tty);
-       if (minor == GB_NUM_MINORS) {
-               dev_err(&gdev->dev, "no more free minor numbers\n");
-               return -ENODEV;
+       if (minor < 0) {
+               if (minor == -ENOSPC) {
+                       dev_err(&gdev->dev, "no more free minor numbers\n");
+                       return -ENODEV;
+               }
+               return minor;
        }
 
        gb_tty->minor = minor;