aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2012-09-21 14:48:05 -0400
committerLinus Torvalds <torvalds@linux-foundation.org>2012-09-21 14:48:05 -0400
commite05e279e6fc940a2adb9d4d4bf2b814dfc286176 (patch)
tree2dfeef2f02576933e752d1fb40e21d810e224a58
parent36048853c5257a7b6df346b83758ffa776a59e9f (diff)
debugfs: fix u32_array race in format_array_alloc
The format_array_alloc() function is fundamentally racy, in that it prints the array twice: once to figure out how much space to allocate for the buffer, and the second time to actually print out the data. If any of the array contents changes in between, the allocation size may be wrong, and the end result may be truncated in odd ways. Just don't do it. Allocate a maximum-sized array up-front, and just format the array contents once. The only user of the u32_array interfaces is the Xen spinlock statistics code, and it has 31 entries in the arrays, so the maximum size really isn't that big, and the end result is much simpler code without the bug. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--fs/debugfs/file.c57
1 files changed, 23 insertions, 34 deletions
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index a09d3c0aad68..c5ca6ae5a30c 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -526,55 +526,44 @@ struct array_data {
526 u32 elements; 526 u32 elements;
527}; 527};
528 528
529static size_t format_array(char *buf, size_t bufsize, const char *fmt, 529static size_t u32_format_array(char *buf, size_t bufsize,
530 u32 *array, u32 array_size) 530 u32 *array, int array_size)
531{ 531{
532 size_t ret = 0; 532 size_t ret = 0;
533 u32 i;
534 533
535 for (i = 0; i < array_size; i++) { 534 while (--array_size >= 0) {
536 size_t len; 535 size_t len;
536 char term = array_size ? ' ' : '\n';
537 537
538 len = snprintf(buf, bufsize, fmt, array[i]); 538 len = snprintf(buf, bufsize, "%u%c", *array++, term);
539 len++; /* ' ' or '\n' */
540 ret += len; 539 ret += len;
541 540
542 if (buf) { 541 buf += len;
543 buf += len; 542 bufsize -= len;
544 bufsize -= len;
545 buf[-1] = (i == array_size-1) ? '\n' : ' ';
546 }
547 } 543 }
548
549 ret++; /* \0 */
550 if (buf)
551 *buf = '\0';
552
553 return ret;
554}
555
556static char *format_array_alloc(const char *fmt, u32 *array,
557 u32 array_size)
558{
559 size_t len = format_array(NULL, 0, fmt, array, array_size);
560 char *ret;
561
562 ret = kmalloc(len, GFP_KERNEL);
563 if (ret == NULL)
564 return NULL;
565
566 format_array(ret, len, fmt, array, array_size);
567 return ret; 544 return ret;
568} 545}
569 546
570static int u32_array_open(struct inode *inode, struct file *file) 547static int u32_array_open(struct inode *inode, struct file *file)
571{ 548{
572 struct array_data *data = inode->i_private; 549 struct array_data *data = inode->i_private;
573 550 int size, elements = data->elements;
574 file->private_data = format_array_alloc("%u", data->array, 551 char *buf;
575 data->elements); 552
576 if (!file->private_data) 553 /*
554 * Max size:
555 * - 10 digits + ' '/'\n' = 11 bytes per number
556 * - terminating NUL character
557 */
558 size = elements*11;
559 buf = kmalloc(size+1, GFP_KERNEL);
560 if (!buf)
577 return -ENOMEM; 561 return -ENOMEM;
562 buf[size] = 0;
563
564 file->private_data = buf;
565 u32_format_array(buf, size, data->array, data->elements);
566
578 return nonseekable_open(inode, file); 567 return nonseekable_open(inode, file);
579} 568}
580 569