Skip Menu |

This queue is for tickets about the YAML-Syck CPAN distribution.

Report information
The Basics
Id: 61562
Status: resolved
Priority: 0/
Queue: YAML-Syck
People
Owner: Nobody in particular
Requestors: stu-rt-cpan [...] spacehopper.org
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 1.30

History Show all quoted text
  Wed Sep 22 07:08:55 2010 stu-rt-cpan [...] spacehopper.org - Ticket created
Subject: Patch: use calloc in S_ALLOC_N
Date: Wed, 22 Sep 2010 12:08:38 +0100
To: bug-YAML-Syck [...] rt.cpan.org
From: Stuart Henderson <stu-rt-cpan [...] spacehopper.org>
OpenBSD's malloc has a mode to fill newly allocated space with junk bytes (just a sequence of 0xd0), useful to confirm that software correctly zeroes out any space that it wants to use. It's low-overhead so many people enable it by default. When this is used, a segfault is seen as perl's Perl_is_utf8_string runs off the end of the buffer looking for the end of the string: #0 0x0dd1a8d5 in Perl_is_utf8_string (s=0x8624ae80 ' <repeats 128 times>, ' <repeats 72 times>..., len=0) at /usr/src/gnu/usr.bin/perl/utf8.c:285 285 const U8* const send = s + (len ? len : strlen((const char *)s)); (gdb) x/16x s 0x8624ae80: 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0x8624ae90: 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0x8624aea0: 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0x8624aeb0: 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 0xd0d0d0d0 (gdb) bt #0 0x0dd1a8d5 in Perl_is_utf8_string (s=0x8624ae80 ' <repeats 128 times>, ' <repeats 72 times>..., len=0) at /usr/src/gnu/usr.bin/perl/utf8.c:285 #1 0x0867a623 in yaml_syck_parser_handler (p=0x8624a680, n=0x8756e900) at perl_syck.h:321 #2 0x0866a068 in syck_hdlr_add_node (p=0x8624a680, n=0x8756e900) at handler.c:19 #3 0x08669891 in syckparse (parser=0x8624a680) at gram.y:381 #4 0x0866f9e3 in syck_parse (p=0x8624a680) at syck_.c:492 #5 0x0867b971 in LoadYAML ( s=0x8a84c000 "--- \n_client:00:04:20:07:b4:63: \n _ts_activeFont: 1248477059\n _ts_activeFont_curr: 1272825909\n _ts_alarmDefa ultVolume: -1\n _ts_alarmSnoozeSeconds: -1\n _ts_alarmTimeoutSeconds: -1\n _ts_alarmfades"...) at perl_syck.h:654 #6 0x0867ddb9 in XS_YAML__Syck_LoadYAML (cv=0x86291610) at Syck.c:78 #7 0x0dc6fda9 in Perl_pp_entersub () at /usr/src/gnu/usr.bin/perl/pp_hot.c:2888 #8 0x0dcadd15 in Perl_runops_standard () at /usr/src/gnu/usr.bin/perl/run.c:40 #9 0x0dca9a53 in Perl_call_sv (sv=0x8245d140, flags=6) at /usr/src/gnu/usr.bin/perl/perl.c:2727 #10 0x0dcacf4b in Perl_call_list (oldscope=2, paramList=0x82332340) at /usr/src/gnu/usr.bin/perl/perl.c:5269 #11 0x0dc9727e in S_process_special_blocks (fullname=0x2dc5035c "\020h\"\205", gv=0x82332320, cv=0x8245d140) at /usr/src/gnu/usr.bin/perl/op.c:5864 #12 0x0dc96b7d in Perl_newATTRSUB (floor=39, o=0x2dc5038c, proto=0x0, attrs=0x0, block=0x7f3b2c60) at /usr/src/gnu/usr.bin/perl/op.c:5835 #13 0x0dc937ef in Perl_utilize (aver=1, floor=39, version=0x7fb7fc00, idop=0x7fb7fbc0, arg=0x0) at /usr/src/gnu/usr.bin/perl/op.c:3878 #14 0x0dcce417 in Perl_yyparse () at perly.y:659 #15 0x0dca86c5 in S_parse_body (env=0x0, xsinit=0x1c000bc0 <xs_init>) at /usr/src/gnu/usr.bin/perl/perl.c:2279 #16 0x0dca807a in perl_parse (my_perl=0x7e4a4030, xsinit=0x1c000bc0 <xs_init>, argc=3, argv=0xcfbdfb3c, env=0x0) at /usr/src/gnu/usr.bin/perl/perl.c:1687 #17 0x1c000b67 in main () I don't have time to track it down further but the following patch uses a sledgehammer approach of calloc for all S_ALLOC_N to make sure the memory is clear, fixing this problem. --- syck.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/syck.h b/syck.h index 90527cb..72c682b 100644 --- a/syck.h +++ b/syck.h @@ -75,7 +75,7 @@ extern "C" { #define ALLOC_CT 8 #define SYCK_BUFFERSIZE 4096 -#define S_ALLOC_N(type,n) (type*)malloc(sizeof(type)*(n)) +#define S_ALLOC_N(type,n) (type*)calloc((n),sizeof(type)) #define S_ALLOC(type) (type*)malloc(sizeof(type)) #define S_REALLOC_N(var,type,n) (var)=(type*)realloc((char*)(var),sizeof(type)*(n)) #define S_FREE(n) free(n); n = NULL; -- 1.7.1
  Thu Sep 23 16:40:45 2010 avar [...] cpan.org - Correspondence added
On 2010-09-22T07:08:55-04:00, stu-rt-cpan@spacehopper.org wrote: Show quoted text
> OpenBSD's malloc has a mode to fill newly allocated space with junk > bytes (just a sequence of 0xd0), useful to confirm that software > correctly zeroes out any space that it wants to use. It's low-overhead > so many people enable it by default. When this is used, a segfault is > seen as perl's Perl_is_utf8_string runs off the end of the buffer > looking for the end of the string: > > I don't have time to track it down further but the following patch > uses a sledgehammer approach of calloc for all S_ALLOC_N to make sure > the memory is clear, fixing this problem. > > --- > syck.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/syck.h b/syck.h > index 90527cb..72c682b 100644 > --- a/syck.h > +++ b/syck.h > @@ -75,7 +75,7 @@ extern "C" { > > #define ALLOC_CT 8 > #define SYCK_BUFFERSIZE 4096 > -#define S_ALLOC_N(type,n) (type*)malloc(sizeof(type)*(n)) > +#define S_ALLOC_N(type,n) (type*)calloc((n),sizeof(type)) > #define S_ALLOC(type) (type*)malloc(sizeof(type)) > #define S_REALLOC_N(var,type,n) > (var)=(type*)realloc((char*)(var),sizeof(type)*(n)) > #define S_FREE(n) free(n); n = NULL;
Using calloc just hides the problem, which presumably is that we have an off-by-one error somwhere so we end up with a copied string that isn't NULL delimited. Can you provide data needed to reproduce this, i.e. the YAML file and whatever YAML::Syck options you used to invoke it?
  Thu Sep 23 16:40:47 2010 The RT System itself - Status changed from 'new' to 'open'
  Thu Mar 03 11:19:08 2016 RURBAN [...] cpan.org - Correspondence added
On Thu Sep 23 16:40:45 2010, AVAR wrote: Show quoted text
> Using calloc just hides the problem, which presumably is that we have an > off-by-one error somwhere so we end up with a copied string that isn't > NULL delimited. > > Can you provide data needed to reproduce this, i.e. the YAML file and > whatever YAML::Syck options you used to invoke it?
1. I do not agree ignoring a SEGV fix for not finding the root cause for 6 years. 2. Looking at the backtrace the reason and fix is trivial to find. It fails with len=0 --- perl_common.h +++ perl_common.h @@ -60,6 +60,7 @@ SV* perl_syck_lookup_sym( SyckParser *p, SYMID v) { #ifdef SvUTF8_on #define CHECK_UTF8 \ if (((struct parser_xtra *)p->bonus)->implicit_unicode \ + && n->data.str->len \ && is_utf8_string((U8*)n->data.str->ptr, n->data.str->len)) \ SvUTF8_on(sv); -- Reini Urban
  Mon Apr 17 20:34:15 2017 TODDR [...] cpan.org - Correspondence added
This will be in 1.30
  Mon Apr 17 20:34:21 2017 TODDR [...] cpan.org - Status changed from 'open' to 'patched'
  Sun Apr 23 22:34:18 2017 TODDR [...] cpan.org - Status changed from 'patched' to 'resolved'
  Sun Apr 23 22:34:18 2017 TODDR [...] cpan.org - Fixed in 1.30 added