bugGNU nano - Bugs: bug #56188, Ctrl+T crashes nano when the...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #56188: Ctrl+T crashes nano when the 'spell' program is missing

Submitter:  Aliaksei Sakovets <sakovecx>
Submitted:  Sun 21 Apr 2019 04:12:14 PM UTC
   
 
Severity:  5 - Blocker Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

Jump to the original submission

Thu 25 Apr 2019 05:19:04 PM UTC, comment #12: 

The fix was released in version 4.2.

Benno Schulenberg <bens>
Group administrator
Tue 23 Apr 2019 10:09:45 AM UTC, comment #11: 

Changed all to const as suggested, commit fcad442d.

Benno Schulenberg <bens>
Group administrator
Mon 22 Apr 2019 09:14:34 PM UTC, comment #10: 

If you are comfortable with a leak you should also be comfortable documenting it in the source.

result_msg, do_int_speller() and do_alt_speller() should be changed back to const char . And the call to invocation_error() should either be cast to const char with a note about the leak, or the function should return const char * with a leak note at the function's declaration.

Otherwise future code involving those functions might create additional leaks or crashes elsewhere.

Brand Huntsman <brand>
Mon 22 Apr 2019 10:49:47 AM UTC, comment #9: 

Fixed in git, commit 493f2155.  Thanks for reporting.

Benno Schulenberg <bens>
Group administrator
Mon 22 Apr 2019 09:16:59 AM UTC, comment #8: 

"You should"?  Stop telling me what to do.

I don't want to wrap all the returned strings in malloc calls, becaue it's ugly.  I will just let the invocation error from the alternate speller leak.  No big deal.

Now I understand why the tested systems didn't crash when 'spell' was missing: those systems are localized to Dutch and seemingly gettext() returns an allocated string there.  Because when I type ^T there, nano does not crash.  But when I start nano preceded with LANGUAGE=en, ^T does crash.

The message "Error invoking "spell"" does not leak because it is a fixed string.  It is produced by the integrated speller.  Only the alternate speller sometimes returns an allocated string that could be freed.  It's not worth the trouble freeing it: it's a small piece of memory, and most likely the user will immediately quit nano to edit their nanorc file or their command line.  So, let it be.

Benno Schulenberg <bens>
Group administrator
Mon 22 Apr 2019 08:35:49 AM UTC, comment #7: 

You should wrap all returned strings in both functions with an alloc and then free in do_spell(). They are error cases so allocation overhead doesn't matter, but without that free, the allocation in invocation_error() will leak.

Brand Huntsman <brand>
Mon 22 Apr 2019 08:12:12 AM UTC, comment #6: 

Hmm.  When I added that free(), I thought it might go wrong when a fixed message like "Could not create pipe" was returned.  I tested that then and it worked fine.  But I must have done something wrong, because I tested it again now, and it fails.  :|  This failure I can comprehend.  But I can't understand why (when the 'spell' program is missing) the "Error invoking ..." message cannot be freed -- it is an allocated string!  But when it is not freed, valgrind does not report any leakage.  :|  Freeing the result_msg when linter invocation fails, gives no problem, though.  So... I don't get it.

Anyway, attached patch removes the offending free().

(file #46809)

Benno Schulenberg <bens>
Group administrator
Mon 22 Apr 2019 07:55:42 AM UTC, comment #5: 

do_spell() is freeing static strings returned by do_int_speller() and do_alt_speller().

f645009a5ee3f987c43abfd78e3dae90ffa04d55

Brand Huntsman <brand>
Mon 22 Apr 2019 06:06:24 AM UTC, comment #4: 

Also happens on Gentoo. Freeing result_msg at end of do_spell throws a SIGABRT with "munmap_chunk(): invalid pointer". I'll finish tracking it down later today or tomorrow.

Brand Huntsman <brand>
Sun 21 Apr 2019 06:39:27 PM UTC, comment #3: 

Well, I compiled several versions of nano from sources. In 4.1 the bug still persists, but 4.0 and 3.0 work just fine.

Aliaksei Sakovets <sakovecx>
Sun 21 Apr 2019 06:08:45 PM UTC, comment #2: 

It still crashes with --ignore flag. I`m using nano 4.1 from Arch Linux official repository.

Aliaksei Sakovets <sakovecx>
Sun 21 Apr 2019 05:04:48 PM UTC, comment #1: 

Thanks for reporting.  However, I cannot reproduce.

Which version of nano are you using?
What happens when you run 'nano --ignore some_file' and press Ctrl+T?

Benno Schulenberg <bens>
Group administrator
Sun 21 Apr 2019 04:12:14 PM UTC, original submission:  

To reproduce, run "nano some_file" and press Ctrl-t.
Nano crashes with the following output:
 Sorry! Nano crashed!  Code: 6.  Please report a bug.

Aliaksei Sakovets <sakovecx>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by brand (Posted a comment)
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by sakovecx (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-25 bens Open/ClosedOpen Closed
    2019-04-22 bens StatusIn Progress Fixed
    2019-04-22 bens Attached File- Added 0001-speller-do-not-crash-by-trying-to-free-something-tha.patch, #46809
        Severity3 - Normal 5 - Blocker
        StatusNone In Progress
        SummaryCtrl-t crashes nano Ctrl+T crashes nano when the 'spell' program is missing
    2019-04-21 bens Assigned toNone bens

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code