Jan Beulich
2012-Jan-24 16:44 UTC
[PATCH] linux-2.6.18/xenoprof: consolidate read/write of active/passive domain IDs
This doesn''t just fold redundant code, but also fixes the potential for
a buffer overrun: While active_domains[] was properly sized (matching
the main loop in adomain_write()), passive_domains[] was declared one
entry too short.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/drivers/oprofile/oprofile_files.c
+++ b/drivers/oprofile/oprofile_files.c
@@ -126,13 +126,22 @@ static const struct file_operations dump
#define TMPBUFSIZE 512
-static unsigned int adomains = 0;
-static int active_domains[MAX_OPROF_DOMAINS + 1];
-static DEFINE_MUTEX(adom_mutex);
+struct domain_data {
+ unsigned int nr;
+ int ids[MAX_OPROF_DOMAINS + 1];
+ struct mutex mutex;
+ int (*set)(int[], unsigned int);
+};
+#define DEFINE_DOMAIN_DATA(what) \
+ struct domain_data what##_domains = { \
+ .mutex = __MUTEX_INITIALIZER(what##_domains.mutex), \
+ .set = oprofile_set_##what \
+ }
-static ssize_t adomain_write(struct file * file, char const __user * buf,
- size_t count, loff_t * offset)
+static ssize_t domain_write(struct file *filp, char const __user *buf,
+ size_t count, loff_t *offset)
{
+ struct domain_data *dom = filp->private_data;
char *tmpbuf;
char *startp, *endp;
int i;
@@ -153,7 +162,7 @@ static ssize_t adomain_write(struct file
}
tmpbuf[count] = 0;
- mutex_lock(&adom_mutex);
+ mutex_lock(&dom->mutex);
startp = tmpbuf;
/* Parse one more than MAX_OPROF_DOMAINS, for easy error checking */
@@ -163,31 +172,32 @@ static ssize_t adomain_write(struct file
break;
while (ispunct(*endp) || isspace(*endp))
endp++;
- active_domains[i] = val;
- if (active_domains[i] != val)
+ dom->ids[i] = val;
+ if (dom->ids[i] != val)
/* Overflow, force error below */
i = MAX_OPROF_DOMAINS + 1;
startp = endp;
}
/* Force error on trailing junk */
- adomains = *startp ? MAX_OPROF_DOMAINS + 1 : i;
+ dom->nr = *startp ? MAX_OPROF_DOMAINS + 1 : i;
kfree(tmpbuf);
- if (adomains > MAX_OPROF_DOMAINS
- || oprofile_set_active(active_domains, adomains)) {
- adomains = 0;
+ if (dom->nr > MAX_OPROF_DOMAINS
+ || dom->set(dom->ids, dom->nr)) {
+ dom->nr = 0;
retval = -EINVAL;
}
- mutex_unlock(&adom_mutex);
+ mutex_unlock(&dom->mutex);
return retval;
}
-static ssize_t adomain_read(struct file * file, char __user * buf,
- size_t count, loff_t * offset)
+static ssize_t domain_read(struct file *filp, char __user *buf,
+ size_t count, loff_t *offset)
{
- char * tmpbuf;
+ struct domain_data *dom = filp->private_data;
+ char *tmpbuf;
size_t len;
int i;
ssize_t retval;
@@ -195,18 +205,18 @@ static ssize_t adomain_read(struct file
if (!(tmpbuf = kmalloc(TMPBUFSIZE, GFP_KERNEL)))
return -ENOMEM;
- mutex_lock(&adom_mutex);
+ mutex_lock(&dom->mutex);
len = 0;
- for (i = 0; i < adomains; i++)
+ for (i = 0; i < dom->nr; i++)
len += snprintf(tmpbuf + len,
len < TMPBUFSIZE ? TMPBUFSIZE - len : 0,
- "%u ", active_domains[i]);
+ "%u ", dom->ids[i]);
WARN_ON(len > TMPBUFSIZE);
if (len != 0 && len <= TMPBUFSIZE)
tmpbuf[len-1] = ''\n'';
- mutex_unlock(&adom_mutex);
+ mutex_unlock(&dom->mutex);
retval = simple_read_from_buffer(buf, count, offset, tmpbuf, len);
@@ -214,103 +224,32 @@ static ssize_t adomain_read(struct file
return retval;
}
+static DEFINE_DOMAIN_DATA(active);
-static const struct file_operations active_domain_ops = {
- .read = adomain_read,
- .write = adomain_write,
-};
-
-static unsigned int pdomains = 0;
-static int passive_domains[MAX_OPROF_DOMAINS];
-static DEFINE_MUTEX(pdom_mutex);
-
-static ssize_t pdomain_write(struct file * file, char const __user * buf,
- size_t count, loff_t * offset)
+static int adomain_open(struct inode *inode, struct file *filp)
{
- char *tmpbuf;
- char *startp, *endp;
- int i;
- unsigned long val;
- ssize_t retval = count;
-
- if (*offset)
- return -EINVAL;
- if (count > TMPBUFSIZE - 1)
- return -EINVAL;
-
- if (!(tmpbuf = kmalloc(TMPBUFSIZE, GFP_KERNEL)))
- return -ENOMEM;
-
- if (copy_from_user(tmpbuf, buf, count)) {
- kfree(tmpbuf);
- return -EFAULT;
- }
- tmpbuf[count] = 0;
-
- mutex_lock(&pdom_mutex);
-
- startp = tmpbuf;
- /* Parse one more than MAX_OPROF_DOMAINS, for easy error checking */
- for (i = 0; i <= MAX_OPROF_DOMAINS; i++) {
- val = simple_strtoul(startp, &endp, 0);
- if (endp == startp)
- break;
- while (ispunct(*endp) || isspace(*endp))
- endp++;
- passive_domains[i] = val;
- if (passive_domains[i] != val)
- /* Overflow, force error below */
- i = MAX_OPROF_DOMAINS + 1;
- startp = endp;
- }
- /* Force error on trailing junk */
- pdomains = *startp ? MAX_OPROF_DOMAINS + 1 : i;
-
- kfree(tmpbuf);
-
- if (pdomains > MAX_OPROF_DOMAINS
- || oprofile_set_passive(passive_domains, pdomains)) {
- pdomains = 0;
- retval = -EINVAL;
- }
-
- mutex_unlock(&pdom_mutex);
- return retval;
+ filp->private_data = &active_domains;
+ return 0;
}
-static ssize_t pdomain_read(struct file * file, char __user * buf,
- size_t count, loff_t * offset)
-{
- char * tmpbuf;
- size_t len;
- int i;
- ssize_t retval;
-
- if (!(tmpbuf = kmalloc(TMPBUFSIZE, GFP_KERNEL)))
- return -ENOMEM;
-
- mutex_lock(&pdom_mutex);
-
- len = 0;
- for (i = 0; i < pdomains; i++)
- len += snprintf(tmpbuf + len,
- len < TMPBUFSIZE ? TMPBUFSIZE - len : 0,
- "%u ", passive_domains[i]);
- WARN_ON(len > TMPBUFSIZE);
- if (len != 0 && len <= TMPBUFSIZE)
- tmpbuf[len-1] = ''\n'';
-
- mutex_unlock(&pdom_mutex);
+static const struct file_operations active_domain_ops = {
+ .open = adomain_open,
+ .read = domain_read,
+ .write = domain_write,
+};
- retval = simple_read_from_buffer(buf, count, offset, tmpbuf, len);
+static DEFINE_DOMAIN_DATA(passive);
- kfree(tmpbuf);
- return retval;
+static int pdomain_open(struct inode *inode, struct file *filp)
+{
+ filp->private_data = &passive_domains;
+ return 0;
}
static const struct file_operations passive_domain_ops = {
- .read = pdomain_read,
- .write = pdomain_write,
+ .open = pdomain_open,
+ .read = domain_read,
+ .write = domain_write,
};
void oprofile_create_files(struct super_block * sb, struct dentry * root)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel