Merge lp:~vlad-lesin/percona-server/5.6-bug-1351148 into lp:percona-server/5.6

Proposed by Vlad Lesin on 2014-09-23
Status: Merged
Approved by: Laurynas Biveinis on 2014-12-02
Approved revision: 661
Merged at revision: 705
Proposed branch: lp:~vlad-lesin/percona-server/5.6-bug-1351148
Merge into: lp:percona-server/5.6
Diff against target: 69 lines (+38/-3)
2 files modified
mysys/lf_alloc-pin.c (+10/-1)
mysys/my_thr_init.c (+28/-2)
To merge this branch: bzr merge lp:~vlad-lesin/percona-server/5.6-bug-1351148
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Approve on 2014-12-02
Registry Administrators 2014-09-23 Pending
Review via email: mp+235721@code.launchpad.net

Description of the change

Bug #1351128 fix.

  The bug's summary:

  1) The value in pins->stack_ends_here is actually incorrect. This value is
  calculated when the thread is initialized in my_thread_init(). The problem is
  that the value doesn't take into account existing stack usage and just adds the
  thread stack size to calculate the beginning of the stack.

  2) _lf_pinbox_real_free() is using alloca() in a very unsafe way. alloca()
  should only be used by leaf functions, since you cannot predict the amount of
  stack space that non-leaf functions will use. As pins->stack_ends_here is
  calculated in wrong way and there is no window for the qsort() which is invoked
  from _lf_pinbox_real_free() after alloca(), stack overflow can take place.

  The solution:

  1) Take into accout the current stack offset when stack size is calculated
  in my_thread_init();

  2) Don't allocate the whole free stack space in _lf_pinbox_real_free(),
  take some window for qsort();

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/714/

To post a comment you must log in.
review: Needs Fixing

5.6 not merged from the last 5.5 push?

review: Needs Fixing
Vlad Lesin (vlad-lesin) wrote :

> 5.6 not merged from the last 5.5 push?

It looks like I forgot to push the last version. Have just pushed.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'mysys/lf_alloc-pin.c'
2--- mysys/lf_alloc-pin.c 2014-12-01 07:53:48 +0000
3+++ mysys/lf_alloc-pin.c 2014-12-02 14:03:08 +0000
4@@ -105,6 +105,13 @@
5
6 #define LF_PINBOX_MAX_PINS 65536
7
8+/*
9+ When allocation on stack happens leave this
10+ amount of stack memory for further functions
11+ call.
12+*/
13+#define LF_PINBOX_EXTRA_STACK_SPACE 8192
14+
15 static void _lf_pinbox_real_free(LF_PINS *pins);
16
17 /*
18@@ -349,7 +356,9 @@
19 {
20 int alloca_size= sizeof(void *)*LF_PINBOX_PINS*npins;
21 /* create a sorted list of pinned addresses, to speed up searches */
22- if (available_stack_size(&pinbox, *pins->stack_ends_here) > alloca_size)
23+ if (available_stack_size(&pinbox, *pins->stack_ends_here) >
24+ /* leave LF_PINBOX_EXTRA_STACK_SPACE bytes for qsort() call */
25+ (alloca_size + LF_PINBOX_EXTRA_STACK_SPACE))
26 {
27 struct st_harvester hv;
28 addr= (void **) alloca(alloca_size);
29
30=== modified file 'mysys/my_thr_init.c'
31--- mysys/my_thr_init.c 2014-12-01 07:53:48 +0000
32+++ mysys/my_thr_init.c 2014-12-02 14:03:08 +0000
33@@ -297,8 +297,34 @@
34 mysql_mutex_init(key_my_thread_var_mutex, &tmp->mutex, MY_MUTEX_INIT_FAST);
35 mysql_cond_init(key_my_thread_var_suspend, &tmp->suspend, NULL);
36
37- tmp->stack_ends_here= (char*)&tmp +
38- STACK_DIRECTION * (long)my_thread_stack_size;
39+#if defined(_GNU_SOURCE) && (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)
40+ {
41+ pthread_attr_t attr;
42+ char* stack_addr;
43+ size_t stack_size;
44+
45+ if (pthread_attr_init(&attr) ||
46+ pthread_getattr_np(tmp->pthread_self, &attr) ||
47+ pthread_attr_getstack(&attr, (void**)&stack_addr, &stack_size))
48+ {
49+ error= 1;
50+ goto end;
51+ }
52+ tmp->stack_ends_here= stack_addr;
53+
54+ if (pthread_attr_destroy(&attr))
55+ {
56+ error= 1;
57+ goto end;
58+ }
59+ }
60+#else //defined(_GNU_SOURCE) && (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)
61+ /* assuming tmp is located within the first page of the stack */
62+ tmp->stack_ends_here= (void **)(((long long)&tmp & (~((long long)my_getpagesize() - 1))) +
63+ ((STACK_DIRECTION == 1) ?
64+ my_thread_stack_size :
65+ my_getpagesize() - my_thread_stack_size));
66+#endif //defined(_GNU_SOURCE) && (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)
67
68 mysql_mutex_lock(&THR_LOCK_threads);
69 tmp->id= ++thread_id;

Subscribers

People subscribed via source and target branches