[MDEV-15795] Stack exceeded if pthread_attr_setstacksize(&thr_attr,8196) succeeds Created: 2018-04-06 Updated: 2022-10-22 Resolved: 2022-10-22 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Server |
| Affects Version/s: | 10.2 |
| Fix Version/s: | 10.3.37, 10.4.27, 10.5.18, 10.6.11, 10.7.7, 10.8.6, 10.9.4 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Vasil (Inactive) | Assignee: | Sergei Golubchik |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Description |
|
mysys/thr_timer.c contains this code:
On Linux the pthread_attr_setstacksize() call on line 88 fails due to `EINVAL The stack size is less than PTHREAD_STACK_MIN (16384) bytes.`, the error is being ignored by the code and so the fact that the thread actually uses more than 8196 bytes of stack remains unnoticed. On FreeBSD the call is accepted and the thread's stack size is indeed capped to 8196 bytes (btw, 2^13 is 8192, why 8196 is used!?). Then the stack is exceeded and a crash occurs during e.g. `mysqld --help --verbose`. Apply this patch to demonstrate the problem, either by printing the bytes that were overwritten beyond the 8196-th byte or by causing a segfault the moment the overflow occurs:
|
| Comments |
| Comment by Vasil (Inactive) [ 2018-04-06 ] |
|
Notice: on amd64 architectures the stack grows from big addresses to small addresses, so the overwritten bytes are before the supplied buffer. Why is `pthread_attr_setstacksize(&thr_attr,8196);` needed in the first place? I guess the fix of this bug is to just remove it. Capping the stack without a mechanism to catch overflow in a useful way leads to hard to diagnose crashes. |
| Comment by Daniel Black [ 2018-04-06 ] |
|
Given its 18+ year old introduction (which I think is where init_thr_timer was copied from), I suspect the original reason isn't valid any more. |
| Comment by Vasil (Inactive) [ 2018-05-03 ] |
|
Hi, I think the fix for this is to just remove the pthread_attr_setstacksize() call. |
| Comment by Vladislav Vaintroub [ 2022-10-21 ] |
|
Hi Serg, the patch looks good to me, ok to push. |