| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <3a932c55376e57a96881a654a0f6c3e6837f400e.1305103212.git.mfm@muteddisk.com>
Date: Wed, 11 May 2011 01:54:18 -0700
From: matt mooney <mfmooney@...il.com>
To: Greg Kroah-Hartman <greg@...ah.com>
Cc: linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: [PATCH 06/12] staging: usbip: stub_main.c: code cleanup
Reorder functions; remove match_find() and replace with
get_busid_idx(); change other functions to use get_busid_idx(); and
code cleanup in the other functions.
Signed-off-by: matt mooney <mfm@...eddisk.com>
---
I apologize for this one. I realize that I should have split it into two with
the reorder being separate.
Also, it seems as if there is a synchronization problem carried over from the
original code in get_busid_priv(). An address into the busid_table is returned
and then the element is accessed and modified elsewhere. Right? Or am I missing
something.
drivers/staging/usbip/stub_main.c | 191 +++++++++++++++++++------------------
1 files changed, 96 insertions(+), 95 deletions(-)
diff --git a/drivers/staging/usbip/stub_main.c b/drivers/staging/usbip/stub_main.c
index 5de33c2..d918c49 100644
--- a/drivers/staging/usbip/stub_main.c
+++ b/drivers/staging/usbip/stub_main.c
@@ -35,112 +35,121 @@ struct kmem_cache *stub_priv_cache;
static struct bus_id_priv busid_table[MAX_BUSID];
static spinlock_t busid_table_lock;
-int match_busid(const char *busid)
+static void init_busid_table(void)
{
int i;
- spin_lock(&busid_table_lock);
- for (i = 0; i < MAX_BUSID; i++)
- if (busid_table[i].name[0])
- if (!strncmp(busid_table[i].name, busid, BUSID_SIZE)) {
- /* already registerd */
- spin_unlock(&busid_table_lock);
- return 0;
- }
- spin_unlock(&busid_table_lock);
+ for (i = 0; i < MAX_BUSID; i++) {
+ memset(busid_table[i].name, 0, BUSID_SIZE);
+ busid_table[i].status = STUB_BUSID_OTHER;
+ busid_table[i].interf_count = 0;
+ busid_table[i].sdev = NULL;
+ busid_table[i].shutdown_busid = 0;
+ }
- return 1;
+ spin_lock_init(&busid_table_lock);
}
-struct bus_id_priv *get_busid_priv(const char *busid)
+/*
+ * Find the index of the busid by name.
+ * Must be called with busid_table_lock held.
+ */
+static int get_busid_idx(const char *busid)
{
int i;
+ int idx = -1;
- spin_lock(&busid_table_lock);
for (i = 0; i < MAX_BUSID; i++)
if (busid_table[i].name[0])
if (!strncmp(busid_table[i].name, busid, BUSID_SIZE)) {
- /* already registerd */
- spin_unlock(&busid_table_lock);
- return &(busid_table[i]);
+ idx = i;
+ break;
}
- spin_unlock(&busid_table_lock);
-
- return NULL;
+ return idx;
}
-static ssize_t show_match_busid(struct device_driver *drv, char *buf)
+struct bus_id_priv *get_busid_priv(const char *busid)
{
- int i;
- char *out = buf;
+ int idx;
+ struct bus_id_priv *bid = NULL;
spin_lock(&busid_table_lock);
- for (i = 0; i < MAX_BUSID; i++)
- if (busid_table[i].name[0])
- out += sprintf(out, "%s ", busid_table[i].name);
+ idx = get_busid_idx(busid);
+ if (idx >= 0)
+ bid = &(busid_table[idx]);
spin_unlock(&busid_table_lock);
- out += sprintf(out, "\n");
- return out - buf;
+ return bid;
}
static int add_match_busid(char *busid)
{
int i;
-
- if (!match_busid(busid))
- return 0;
+ int ret = -1;
spin_lock(&busid_table_lock);
+ /* already registered? */
+ if (get_busid_idx(busid) >= 0) {
+ ret = 0;
+ goto out;
+ }
+
for (i = 0; i < MAX_BUSID; i++)
if (!busid_table[i].name[0]) {
strncpy(busid_table[i].name, busid, BUSID_SIZE);
if ((busid_table[i].status != STUB_BUSID_ALLOC) &&
(busid_table[i].status != STUB_BUSID_REMOV))
busid_table[i].status = STUB_BUSID_ADDED;
- spin_unlock(&busid_table_lock);
- return 0;
+ ret = 0;
+ break;
}
+
+out:
spin_unlock(&busid_table_lock);
- return -1;
+ return ret;
}
int del_match_busid(char *busid)
{
- int i;
+ int idx;
+ int ret = -1;
spin_lock(&busid_table_lock);
- for (i = 0; i < MAX_BUSID; i++)
- if (!strncmp(busid_table[i].name, busid, BUSID_SIZE)) {
- /* found */
- if (busid_table[i].status == STUB_BUSID_OTHER)
- memset(busid_table[i].name, 0, BUSID_SIZE);
- if ((busid_table[i].status != STUB_BUSID_OTHER) &&
- (busid_table[i].status != STUB_BUSID_ADDED)) {
- busid_table[i].status = STUB_BUSID_REMOV;
- }
- spin_unlock(&busid_table_lock);
- return 0;
- }
+ idx = get_busid_idx(busid);
+ if (idx < 0)
+ goto out;
+
+ /* found */
+ ret = 0;
+
+ if (busid_table[idx].status == STUB_BUSID_OTHER)
+ memset(busid_table[idx].name, 0, BUSID_SIZE);
+
+ if ((busid_table[idx].status != STUB_BUSID_OTHER) &&
+ (busid_table[idx].status != STUB_BUSID_ADDED))
+ busid_table[idx].status = STUB_BUSID_REMOV;
+
+out:
spin_unlock(&busid_table_lock);
- return -1;
+ return ret;
}
-static void init_busid_table(void)
+static ssize_t show_match_busid(struct device_driver *drv, char *buf)
{
int i;
+ char *out = buf;
- for (i = 0; i < MAX_BUSID; i++) {
- memset(busid_table[i].name, 0, BUSID_SIZE);
- busid_table[i].status = STUB_BUSID_OTHER;
- busid_table[i].interf_count = 0;
- busid_table[i].sdev = NULL;
- busid_table[i].shutdown_busid = 0;
- }
+ spin_lock(&busid_table_lock);
+ for (i = 0; i < MAX_BUSID; i++)
+ if (busid_table[i].name[0])
+ out += sprintf(out, "%s ", busid_table[i].name);
+ spin_unlock(&busid_table_lock);
- spin_lock_init(&busid_table_lock);
+ out += sprintf(out, "\n");
+
+ return out - buf;
}
static ssize_t store_match_busid(struct device_driver *dev, const char *buf,
@@ -162,23 +171,24 @@ static ssize_t store_match_busid(struct device_driver *dev, const char *buf,
strncpy(busid, buf + 4, BUSID_SIZE);
if (!strncmp(buf, "add ", 4)) {
- if (add_match_busid(busid) < 0)
+ if (add_match_busid(busid) < 0) {
return -ENOMEM;
- else {
+ } else {
pr_debug("add busid %s\n", busid);
return count;
}
} else if (!strncmp(buf, "del ", 4)) {
- if (del_match_busid(busid) < 0)
+ if (del_match_busid(busid) < 0) {
return -ENODEV;
- else {
+ } else {
pr_debug("del busid %s\n", busid);
return count;
}
- } else
+ } else {
return -EINVAL;
+ }
}
-static DRIVER_ATTR(match_busid, S_IRUSR|S_IWUSR, show_match_busid,
+static DRIVER_ATTR(match_busid, S_IRUSR | S_IWUSR, show_match_busid,
store_match_busid);
static struct stub_priv *stub_priv_pop_from_listhead(struct list_head *listhead)
@@ -201,84 +211,75 @@ static struct stub_priv *stub_priv_pop(struct stub_device *sdev)
spin_lock_irqsave(&sdev->priv_lock, flags);
priv = stub_priv_pop_from_listhead(&sdev->priv_init);
- if (priv) {
- spin_unlock_irqrestore(&sdev->priv_lock, flags);
- return priv;
- }
+ if (priv)
+ goto done;
priv = stub_priv_pop_from_listhead(&sdev->priv_tx);
- if (priv) {
- spin_unlock_irqrestore(&sdev->priv_lock, flags);
- return priv;
- }
+ if (priv)
+ goto done;
priv = stub_priv_pop_from_listhead(&sdev->priv_free);
- if (priv) {
- spin_unlock_irqrestore(&sdev->priv_lock, flags);
- return priv;
- }
+done:
spin_unlock_irqrestore(&sdev->priv_lock, flags);
- return NULL;
+
+ return priv;
}
void stub_device_cleanup_urbs(struct stub_device *sdev)
{
struct stub_priv *priv;
+ struct urb *urb;
pr_debug("free sdev %p\n", sdev);
while ((priv = stub_priv_pop(sdev))) {
- struct urb *urb = priv->urb;
-
- pr_debug(" free urb %p\n", urb);
+ urb = priv->urb;
+ pr_debug("free urb %p\n", urb);
usb_kill_urb(urb);
kmem_cache_free(stub_priv_cache, priv);
kfree(urb->transfer_buffer);
kfree(urb->setup_packet);
-
usb_free_urb(urb);
}
}
static int __init usb_stub_init(void)
{
- int ret;
+ int ret = 0;
stub_priv_cache = kmem_cache_create("stub_priv",
sizeof(struct stub_priv), 0,
SLAB_HWCACHE_ALIGN, NULL);
-
if (!stub_priv_cache) {
- pr_err("create stub_priv_cache error\n");
+ pr_err("kmem_cache_create failed\n");
return -ENOMEM;
}
ret = usb_register(&stub_driver);
- if (ret) {
+ if (ret < 0) {
pr_err("usb_register failed %d\n", ret);
- goto error_usb_register;
+ goto err_usb_register;
}
- pr_info(DRIVER_DESC " " USBIP_VERSION "\n");
-
- init_busid_table();
-
ret = driver_create_file(&stub_driver.drvwrap.driver,
&driver_attr_match_busid);
-
- if (ret) {
- pr_err("create driver sysfs\n");
- goto error_create_file;
+ if (ret < 0) {
+ pr_err("driver_create_file failed\n");
+ goto err_create_file;
}
- return ret;
-error_create_file:
+ init_busid_table();
+ pr_info(DRIVER_DESC " v" USBIP_VERSION "\n");
+ goto out;
+
+err_create_file:
usb_deregister(&stub_driver);
-error_usb_register:
+err_usb_register:
kmem_cache_destroy(stub_priv_cache);
+out:
return ret;
}
--
1.7.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists