ring-buffer: Use long for nr_pages to avoid overflow failures
authorSteven Rostedt (Red Hat) <rostedt@goodmis.org>
Thu, 12 May 2016 15:01:24 +0000 (11:01 -0400)
committerWilly Tarreau <w@1wt.eu>
Tue, 7 Jun 2016 08:42:54 +0000 (10:42 +0200)
commit 9b94a8fba501f38368aef6ac1b30e7335252a220 upstream.

The size variable to change the ring buffer in ftrace is a long. The
nr_pages used to update the ring buffer based on the size is int. On 64 bit
machines this can cause an overflow problem.

For example, the following will cause the ring buffer to crash:

 # cd /sys/kernel/debug/tracing
 # echo 10 > buffer_size_kb
 # echo 8556384240 > buffer_size_kb

Then you get the warning of:

 WARNING: CPU: 1 PID: 318 at kernel/trace/ring_buffer.c:1527 rb_update_pages+0x22f/0x260

Which is:

  RB_WARN_ON(cpu_buffer, nr_removed);

Note each ring buffer page holds 4080 bytes.

This is because:

 1) 10 causes the ring buffer to have 3 pages.
    (10kb requires 3 * 4080 pages to hold)

 2) (2^31 / 2^10  + 1) * 4080 = 8556384240
    The value written into buffer_size_kb is shifted by 10 and then passed
    to ring_buffer_resize(). 8556384240 * 2^10 = 8761737461760

 3) The size passed to ring_buffer_resize() is then divided by BUF_PAGE_SIZE
    which is 4080. 8761737461760 / 4080 = 2147484672

 4) nr_pages is subtracted from the current nr_pages (3) and we get:
    2147484669. This value is saved in a signed integer nr_pages_to_update

 5) 2147484669 is greater than 2^31 but smaller than 2^32, a signed int
    turns into the value of -2147482627

 6) As the value is a negative number, in update_pages_handler() it is
    negated and passed to rb_remove_pages() and 2147482627 pages will
    be removed, which is much larger than 3 and it causes the warning
    because not all the pages asked to be removed were removed.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=118001
Fixes: 7a8e76a3829f1 ("tracing: unified trace buffer")
Reported-by: Hao Qin <QEver.cn@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
kernel/trace/ring_buffer.c

index cb73c4e0741e309146ba4ff3f9d2fc296a304bcb..6f70235770b94d907c48a7bf4cf62d92740fa0af 100644 (file)
@@ -463,7 +463,7 @@ struct ring_buffer_per_cpu {
        raw_spinlock_t                  reader_lock;    /* serialize readers */
        arch_spinlock_t                 lock;
        struct lock_class_key           lock_key;
-       unsigned int                    nr_pages;
+       unsigned long                   nr_pages;
        struct list_head                *pages;
        struct buffer_page              *head_page;     /* read from head */
        struct buffer_page              *tail_page;     /* write to tail */
@@ -483,7 +483,7 @@ struct ring_buffer_per_cpu {
        u64                             write_stamp;
        u64                             read_stamp;
        /* ring buffer pages to update, > 0 to add, < 0 to remove */
-       int                             nr_pages_to_update;
+       long                            nr_pages_to_update;
        struct list_head                new_pages; /* new pages to add */
        struct work_struct              update_pages_work;
        struct completion               update_done;
@@ -1120,10 +1120,10 @@ static int rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
        return 0;
 }
 
-static int __rb_allocate_pages(int nr_pages, struct list_head *pages, int cpu)
+static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
 {
-       int i;
        struct buffer_page *bpage, *tmp;
+       long i;
 
        for (i = 0; i < nr_pages; i++) {
                struct page *page;
@@ -1160,7 +1160,7 @@ free_pages:
 }
 
 static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
-                            unsigned nr_pages)
+                            unsigned long nr_pages)
 {
        LIST_HEAD(pages);
 
@@ -1185,7 +1185,7 @@ static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 }
 
 static struct ring_buffer_per_cpu *
-rb_allocate_cpu_buffer(struct ring_buffer *buffer, int nr_pages, int cpu)
+rb_allocate_cpu_buffer(struct ring_buffer *buffer, long nr_pages, int cpu)
 {
        struct ring_buffer_per_cpu *cpu_buffer;
        struct buffer_page *bpage;
@@ -1284,8 +1284,9 @@ struct ring_buffer *__ring_buffer_alloc(unsigned long size, unsigned flags,
                                        struct lock_class_key *key)
 {
        struct ring_buffer *buffer;
+       long nr_pages;
        int bsize;
-       int cpu, nr_pages;
+       int cpu;
 
        /* keep it in its own cache line */
        buffer = kzalloc(ALIGN(sizeof(*buffer), cache_line_size()),
@@ -1408,12 +1409,12 @@ static inline unsigned long rb_page_write(struct buffer_page *bpage)
 }
 
 static int
-rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned int nr_pages)
+rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 {
        struct list_head *tail_page, *to_remove, *next_page;
        struct buffer_page *to_remove_page, *tmp_iter_page;
        struct buffer_page *last_page, *first_page;
-       unsigned int nr_removed;
+       unsigned long nr_removed;
        unsigned long head_bit;
        int page_entries;
 
@@ -1629,7 +1630,7 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size,
                        int cpu_id)
 {
        struct ring_buffer_per_cpu *cpu_buffer;
-       unsigned nr_pages;
+       unsigned long nr_pages;
        int cpu, err = 0;
 
        /*
@@ -4607,8 +4608,9 @@ static int rb_cpu_notify(struct notifier_block *self,
        struct ring_buffer *buffer =
                container_of(self, struct ring_buffer, cpu_notify);
        long cpu = (long)hcpu;
-       int cpu_i, nr_pages_same;
-       unsigned int nr_pages;
+       long nr_pages_same;
+       int cpu_i;
+       unsigned long nr_pages;
 
        switch (action) {
        case CPU_UP_PREPARE: