From ff5f0b38228fbf10ff58a530a8a462b3b90cc7ef Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 18 Aug 2014 18:25:11 -0500 Subject: [PATCH] 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 Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/uart-gb.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/staging/greybus/uart-gb.c b/drivers/staging/greybus/uart-gb.c index 13a23cd..0e2507c 100644 --- 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; -- 2.7.4