Skip to content

Fix a crash in String::changeBuffer() #1668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2016
Merged

Fix a crash in String::changeBuffer() #1668

merged 1 commit into from
Feb 26, 2016

Conversation

raheelh
Copy link
Contributor

@raheelh raheelh commented Feb 23, 2016

Calling String::reserve(0) causes a crash if String object is in invalidated state. Per the comment on the method's declaration in WString.h, reserve(0) would validate the string object but in fact it just causes a crash instead of recovering from invalidated strings.

This change fixes the edge case bug in String::changeBuffer() which is the root cause of the crash exposed from String::reserve().

Following test code was used to reproduce the problem and also to validate the fix:

String result;
while(true){ // suppose this is a loop which reads data from a web page.
  char c = 'A'; // suppose this is a byte read from client.
  result += c; // the loop will eventually cause malloc() to fail here
  if (result.c_str()==0) // even if we detect this
  {
    Serial.println("String INVALIDATED!!!!!");
    // attempt to validate string object so we can use it.
    result.reserve(0);   // before fix, this would crash.
    // if reserve(0) worked, the object is validated, i.e. available for use again.
    Serial.println("Trying to empty...."); // try to make use of object to see.
    result=""; 
    Serial.println("Emptied!!!!"); // works!
    break;
  } 
}

Calling String::reserve() causes a crash if String object was in invalidated state. Per the comment on the method's declaration in ESP_SSD1306.h, This method was supposed to recover invalidated strings. This change fixes the edge case bug in String::changeBuffer() which is the root cause of the crash exposed from String::reserve().

Following test code was used to reproduce the problem and also to validate the fix:

String result;
while(true){
  char c = 'A';
  result += c; // the loop will cause malloc() to fail at some point.
  if (result.c_str()==0)
  {
    Serial.println("String INVALIDATED!!!!!");
    result.reserve(0);   // before fix, this would crash.
    Serial.println("Trying to empty....");
    result=""; 
    Serial.println("Emptied!!!!");
    break;
  } 
}
igrr added a commit that referenced this pull request Feb 26, 2016
Fix a crash in String::changeBuffer()
@igrr igrr merged commit 87a7f8e into esp8266:master Feb 26, 2016
@raheelh raheelh deleted the patch-1 branch February 27, 2016 04:16
@igrr
Copy link
Member

igrr commented Feb 28, 2016

umm_realloc will attempt to extend the buffer if it is possible, i.e. the adjacent block is not allocated.
You can check umm_malloc source code (it is included in this repository).

@raheelh
Copy link
Contributor Author

raheelh commented Feb 28, 2016

Makes sense. I just created a pull request with a change to leverage realloc() in this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants