[MXS-350] Return value of realloc must not be assigned to provided pointer. Created: 2015-09-04  Updated: 2017-11-17  Resolved: 2016-06-16

Status: Closed
Project: MariaDB MaxScale
Component/s: Core
Affects Version/s: 1.1.0, 1.2.0, 1.3.0, 1.4.0
Fix Version/s: 2.1.0

Type: Bug Priority: Minor
Reporter: Johan Wikman Assignee: Johan Wikman
Resolution: Done Votes: 0
Labels: None

Issue Links:
Blocks
is blocked by MXS-354 Memory Allocation Closed
is blocked by MXS-1529 OOM: mxs_realloc can be repeated this... Closed
Sprint: 2016-12

 Description   

If realloc fails it returns NULL. Thus, in a situation like

void* p = ...;
 
...;
 
p = realloc(p, ...);

if realloc fails, the previously allocated memory will leak. Realloc must be used like:

void* p = ...;
 
...;
 
void* tmp = realloc(p, ...);
 
 
 
if (tmp) {
 
    p = tmp;
 
} else {
 
    ...
 
}



 Comments   
Comment by Johan Wikman [ 2015-09-05 ]

This cannot be done mechanically, since there is code that simply assumes that memory allocations will not fail. Consequently, larger re-factoring is needed.

Comment by Johan Wikman [ 2015-09-07 ]

Lowering priority to minor.

Not using realloc the way it is supposed to be used is clearly a bug, but one which in practice will not manifest itself. Consequently, this is better handled as part of general refactoring to be done post 1.3.

Comment by Johan Wikman [ 2016-02-29 ]

Again lowering to minor. The usage is wrong but in the environments were MaxScale will be used, malloc, realloc and calloc will in practice not fail but return a valid pointer. Later, when the memory is actually accessed, the OOM killer might kill MaxScale but checking for the return value will not save from that.

Comment by Johan Wikman [ 2016-06-16 ]

All calls to realloc corrected except where the function calling realloc cannot fail, in which case the use of realloc has been replaced with mxs_realloc that takes the process down if the allocation fails.

I.e. grepping for mxs_realloc reveals places that may need refactoring.

Comment by Wagner Bianchi (Inactive) [ 2017-11-16 ]

Another way to generate an error like this one is calling a non-existing configuration file, like below:

[root@maxscale ~]# maxscale -f ~ ~/maxscale_configs/basic.cnf
 
* Error: Failed to start all MaxScale services. Exiting.
 
 
 
MariaDB MaxScale  /var/log/maxscale/maxscale.log  Thu Nov 16 11:35:42 2017
 
----------------------------------------------------------------------------
 
^@^@^@2017-11-16 11:35:42   notice : Working directory: /var/log/maxscale
 
2017-11-16 11:35:42   notice : MariaDB MaxScale 2.1.10 started
 
2017-11-16 11:35:42   notice : MaxScale is running in process 4237
 
2017-11-16 11:35:42   notice : Configuration file: /root
 
2017-11-16 11:35:42   notice : Log directory: /var/log/maxscale
 
2017-11-16 11:35:42   notice : Data directory: /var/lib/maxscale
 
2017-11-16 11:35:42   notice : Module directory: /usr/lib64/maxscale
 
2017-11-16 11:35:42   notice : Service cache: /var/cache/maxscale
 
2017-11-16 11:35:42   notice : Loading /root.
 
2017-11-16 11:38:58   error  : OOM: mxs_realloc
 
2017-11-16 11:38:58   notice : /root.d does not exist, not reading.
 
2017-11-16 11:38:58   notice : No query classifier specified, using default 'qc_sqlite'.
 
2017-11-16 11:38:58   notice : Loaded module qc_sqlite: V1.0.0 from /usr/lib64/maxscale/libqc_sqlite.so
 
2017-11-16 11:38:58   error  : Failed to start all MaxScale services. Exiting.
 
2017-11-16 11:38:58   MariaDB MaxScale is shut down.
 
----------------------------------------------------

Generated at Thu Feb 08 03:58:38 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.