headers
Mike Frysinger | 3 Feb 2006 01:16
Picon
Favicon
Gravatar

Re: Crash report - segfault reproduceable

On Thursday 02 February 2006 18:02, David Lawrence Ramsey wrote:
> No problem.  Would you try getting the latest CVS, applying the
> following patch, running nano WITHOUT the -O/--morespace option (as the
> debugging output is on the line that that option makes part of the edit
> window) so that it crashes, and telling me what results are shown during
> the entire session?  Thanks in advance.

well if i use a terminal size of 81x24 it crashes at startup:
line 1: allocated 83 bytes, index == 88, converted length == 89

also, it'd be easier to capture the output if that statement wrote to 
stderr ...
-mike
Permalink | Reply |
headers
David Lawrence Ramsey | 3 Feb 2006 01:50
Picon

Re: Crash report - segfault reproduceable

Mike Frysinger wrote:
 > On Thursday 02 February 2006 18:02, David Lawrence Ramsey wrote:
 >> No problem.  Would you try getting the latest CVS, applying the
 >> following patch, running nano WITHOUT the -O/--morespace option (as
 >> the debugging output is on the line that that option makes part of
 >> the edit window) so that it crashes, and telling me what results are
 >> shown during the entire session?  Thanks in advance.
 >
 > well if i use a terminal size of 81x24 it crashes at startup:
 > line 1: allocated 83 bytes, index == 88, converted length == 89

I can reproduce this now.  I've been using UTF-8 mode exclusively for
awhile now, and the size of MB_CUR_MAX apparently provides enough of a
buffer that the buffer overflow in display_string() that you're seeing
doesn't happen.  Current CVS allocates COLS characters per line, and the
extra tab on the end overflows it, so adding room for tabsize
characters, as in the attached patch, should fix it.  Please let me know
if it does (and please apply it on top of the other patch so that I can
be sure).

Note that this doesn't fix the problem with the "coretest" file, so it's
most likely a separate problem.

 > also, it'd be easier to capture the output if that statement wrote to
 > stderr ...

True, but it'd be harder to see what was onscreen, which is why I did it
that way.  If the attached patch doesn't work, I'll make a new version
of the other patch.
(Continue reading)

Permalink | Reply |
headers
Mike Frysinger | 3 Feb 2006 02:19
Picon
Favicon
Gravatar

Re: Crash report - segfault reproduceable

On Thursday 02 February 2006 19:50, David Lawrence Ramsey wrote:
> I can reproduce this now.  I've been using UTF-8 mode exclusively for
> awhile now, and the size of MB_CUR_MAX apparently provides enough of a
> buffer that the buffer overflow in display_string() that you're seeing
> doesn't happen.  Current CVS allocates COLS characters per line, and the
> extra tab on the end overflows it, so adding room for tabsize
> characters, as in the attached patch, should fix it.  Please let me know
> if it does (and please apply it on top of the other patch so that I can
> be sure).

ok, the coretest file crashed less often, but here's the output after moving 
to a 90x24 terminal:
line 1: allocated 100 bytes, index == 104, converted length == 105
-mike
Permalink | Reply |
headers
David Lawrence Ramsey | 3 Feb 2006 05:21
Picon

Re: Crash report - segfault reproduceable

Mike Frysinger wrote:

<snip>

 >ok, the coretest file crashed less often, but here's the output after
 >moving to a 90x24 terminal:
 >line 1: allocated 100 bytes, index == 104, converted length == 105

I think I've finally fixed it, since neither of your test cases seem to
crash current CVS anymore.  Could you get current CVS again (note that
the patch won't apply cleanly to it anymore, but that's because I
reworked the memory allocation; sorry for any inconvenience; you can use
--enable-debug instead, since I added an assert that index will never be
out of range of alloc_len) and try it?  Thanks in advance.
Permalink | Reply |
headers
Mike Frysinger | 3 Feb 2006 05:23
Picon
Favicon
Gravatar

Re: Crash report - segfault reproduceable

On Thursday 02 February 2006 23:21, David Lawrence Ramsey wrote:
> I think I've finally fixed it, since neither of your test cases seem to
> crash current CVS anymore.

indeed ... constantly resizing and pounding the arrow keys yields no crash

kudos !
-mike
Permalink | Reply |
headers
David Lawrence Ramsey | 3 Feb 2006 05:49
Picon

Re: Crash report - segfault reproduceable

Mike Frysinger wrote:

<snip>

 > indeed ... constantly resizing and pounding the arrow keys yields no
 > crash
 >
 > kudos !

Thank you.  I'm attaching patches to add the same fix to 1.3.7 through
1.3.10, since this is a longstanding problem.


diff -ur nano-1.3.7/src/winio.c nano-1.3.7-fixed/src/winio.c
--- nano-1.3.7/src/winio.c	2005-04-10 23:51:22.000000000 -0400
+++ nano-1.3.7-fixed/src/winio.c	2006-02-02 23:47:01.000000000 -0500
 <at>  <at>  -2253,11 +2253,22  <at>  <at> 

     assert(column <= start_col);

-    /* Allocate enough space for the entire line.  It should contain
-     * (len + 2) multibyte characters at most. */
-    alloc_len = mb_cur_max() * (len + 2);
+    /* Make sure there's enough room for the initial character, whether
+     * it's a multibyte control character, a non-control multibyte
+     * character, a tab character, or a null terminator.  Rationale:
+     *
+     * multibyte control character followed by a null terminator:
+     *     1 byte ('^') + mb_cur_max() bytes + 1 byte ('\0')
(Continue reading)

Permalink | Reply |
headers
Nick Warne | 3 Feb 2006 08:16

Re: Crash report - segfault reproduceable

On Friday 03 February 2006 04:49, David Lawrence Ramsey wrote:
> Mike Frysinger wrote:
>
> <snip>
>
>  > indeed ... constantly resizing and pounding the arrow keys yields no
>  > crash
>  >
>  > kudos !
>
> Thank you.  I'm attaching patches to add the same fix to 1.3.7 through
> 1.3.10, since this is a longstanding problem.

Good job!  I applied cvs here at home and I cannot get any segfault repeating 
my otherwise 'reproduceable' crashes!  I will repeat at work today, but it 
looks good.

Thank you!

Nick
--

-- 
"Person who say it cannot be done should not interrupt person doing it."
-Chinese Proverb
Permalink | Reply |
headers
Nick Warne | 3 Feb 2006 19:20

Re: Crash report - segfault reproduceable

On Friday 03 February 2006 07:16, Nick Warne wrote:

> > Thank you.  I'm attaching patches to add the same fix to 1.3.7 through
> > 1.3.10, since this is a longstanding problem.
>
> Good job!  I applied cvs here at home and I cannot get any segfault
> repeating my otherwise 'reproduceable' crashes!  I will repeat at work
> today, but it looks good.

Thanks Dave, nano is now good on all machines I could reproduce the segfault.

Good fix!

Nick

--

-- 
"Person who say it cannot be done should not interrupt person doing it."
-Chinese Proverb
Permalink | Reply |
headers
David Lawrence Ramsey | 3 Feb 2006 21:01
Picon

Re: Crash report - segfault reproduceable

Nick Warne wrote:

<snip>

 >> Good job!  I applied cvs here at home and I cannot get any segfault
 >> repeating my otherwise 'reproduceable' crashes!  I will repeat at
 >> work today, but it looks good.
 >
 > Thanks Dave, nano is now good on all machines I could reproduce the
 > segfault.
 >
 > Good fix!

Thank you.  I'm glad I could finally help.
Permalink | Reply |

Gmane