Fix segfault on trying to free an invalid pointer in tokenizer.c
State: closed

This fixes a segmentation fault on an out of memory error in `tokenizer.c` (encountered when loading large JSON files).

The gist of the error is that we're trying to `free()` a pointer that points to a read-only area of memory holding the error message string literal, as illustrated by the example below:

```c
#include<stdio.h>
#include<stdlib.h>


typedef struct Readonly {
    char *message;
} Readonly;


int main() {
    Readonly readonly;

    // This works
    readonly.message = (char*) malloc(200);
    sprintf(readonly.message, "This goes into normal memory\n");

    printf(readonly.message);

    // This fails as we try to free in a read-only memory region.
    readonly.message = "This goes into a read-only data section";

    // Segfault here
    free(readonly.message);
}
```

I will try to add tests that exercise this code path and verify that this solution is correct.



Labels:
name: IO CSV
description: read_csv, to_csv

Comments:
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14696?src=pr) is 85.21% (diff: 100%)
> Merging [#14696](https://codecov.io/gh/pandas-dev/pandas/pull/14696?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **<.01%**

```diff
@@             master     #14696   diff @@
==========================================
  Files           143        143          
  Lines         50787      50804    +17   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43273      43291    +18   
+ Misses         7514       7513     -1   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [f26b049...ef4f736](https://codecov.io/gh/pandas-dev/pandas/compare/f26b049786624ed983f2718687c23e3f1adbb670...ef4f7369074b4ec2db3ef0b4836c3a95a3e4144b?src=pr)
do you have a repro example where this fix is needed?
This will trigger the segfault on a 64-bit Linux machine. I could add a test but it will require quite a bit of memory.
```
import StringIO

import pandas as pd

size = 10 ** 10

csv = StringIO.StringIO('"", "%s"' % ('a' * size))

df = pd.read_csv(csv)
```
so this is *only* hit if the previous allocator actually fails to malloc space. I am not really sure what the defined behavior should then be. You are assuming that you *can* then allocate memory to write the error message, which may or may not be true. it currently stack allocates which should work. 

I think its maybe better to allocate say 200bytes (longest possible message) for error_msg (and maybe for warn). on parser initialization, then we *know* this cannot fail and will be safe.

cc @wesm 
Pre-allocating sounds like a sensible defensive solution, I could probably make that change. We could even allocate the space directly in the struct and save ourselves malloc and free calls.

In the repro example this is triggered when there is still plenty of memory available for the error string, but not enough to complete the `realloc`: I'm guessing it's trying to allocate buffers of some ridiculous size (as the string itself is less than a gigabyte in size).

There is also a separate question if it is desirable behaviour in the tokenizer to explode on such inputs (the example I gave triggers the segfault even on machines with 64 gigabytes of RAM).
@maciejkula let's leave this issue to pre-allocating the buffers for error/warn (or in structure is fine too).

yes that might be an error if its trying to allocate way too much memory (maybe you can find a rationale / reason why its doing that). you can open a separate issue for it.
I had a stab at pre-allocating the buffers. This requires moving to out-of-band communication of errors, as the code was previously checking whether the error message pointer was `NULL`.
can you rebase.
can you rebase
can you update
closing as stale, but if you want to finish up would be great.
if someone wants to take this over would be great. I think just needs a rebase / doc-note.

@dsm054 ?
@jreback : Am I understanding properly : this needs to just be rebased and documented?  (no other functional changes) ?
I can take this to the finish if that's the case.  Just let me know.
i think this is being handled in #17040