From 35ec25a6cf7c2686aa9b6b972403788937b6f6fd Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 17 Jan 2020 16:12:06 +0000 Subject: closes [5d989f9ba3]: avoid segfault by OOM if too many items to sort (would throw an error instead, and use system alloc for long elementArray due to size_t); added test cases covering that (in child process with limited address space, if "prlimit" available) --- generic/tclCmdIL.c | 22 ++++++++++++++++++++-- tests/cmdIL.test | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/generic/tclCmdIL.c b/generic/tclCmdIL.c index b303bb6..630aa70 100644 --- a/generic/tclCmdIL.c +++ b/generic/tclCmdIL.c @@ -3477,6 +3477,7 @@ Tcl_LsortObjCmd( int i, j, index, indices, length, nocase = 0, indexc; int sortMode = SORTMODE_ASCII; Tcl_Obj *resultPtr, *cmdPtr, **listObjPtrs, *listObj, *indexPtr; + size_t elmArrSize; SortElement *elementArray, *elementPtr; SortInfo sortInfo; /* Information about this sort that needs to * be passed to the comparison function. */ @@ -3494,6 +3495,7 @@ Tcl_LsortObjCmd( * The subList array below holds pointers to temporary lists built during * the merge sort. Element i of the array holds a list of length 2**i. */ +# define MAXCALLOC 1024000 # define NUM_LISTS 30 SortElement *subList[NUM_LISTS+1]; @@ -3699,7 +3701,19 @@ Tcl_LsortObjCmd( * begins sorting it into the sublists as it appears. */ - elementArray = (SortElement *) ckalloc( length * sizeof(SortElement)); + elmArrSize = length * sizeof(SortElement); + if (elmArrSize <= MAXCALLOC) { + elementArray = (SortElement *) ckalloc(elmArrSize); + } else { + elementArray = (SortElement *) malloc(elmArrSize); + } + if (!elementArray) { + Tcl_SetObjResult(interp, Tcl_ObjPrintf( + "no enough memory to proccess sort of %d items", length)); + Tcl_SetErrorCode(interp, "TCL", "MEMORY", NULL); + sortInfo.resultCode = TCL_ERROR; + goto done; + } for (i=0; i < length; i++){ if (indexc) { @@ -3802,7 +3816,11 @@ Tcl_LsortObjCmd( } done1: - ckfree((char *)elementArray); + if (elmArrSize <= MAXCALLOC) { + ckfree((char *)elementArray); + } else { + free((char *)elementArray); + } done: if (sortMode == SORTMODE_COMMAND) { diff --git a/tests/cmdIL.test b/tests/cmdIL.test index 6fab269..b743b35 100644 --- a/tests/cmdIL.test +++ b/tests/cmdIL.test @@ -15,6 +15,7 @@ if {[lsearch [namespace children] ::tcltest] == -1} { # Used for constraining memory leak tests testConstraint memory [llength [info commands memory]] +testConstraint prlimit [expr {[testConstraint macOrUnix] && ![catch { exec prlimit -n }]}] test cmdIL-1.1 {Tcl_LsortObjCmd procedure} { list [catch {lsort} msg] $msg @@ -445,6 +446,38 @@ test cmdIL-5.5 {lsort with list style index and sharing} -body { } -result 0 -cleanup { rename test_lsort "" } +test cmdIL-5.7 {lsort memory exhaustion} -constraints {prlimit} -body { + # test it in child process (with limited address space): + set pipe {} + if {[catch { + set pipe [open |[list [interpreter]] r+] + exec prlimit -p [pid $pipe] --as=80000000 + } msg]} { + catch {close $pipe} + tcltest::Skip "prlimit: set of limit is impossible, [regsub {^\s*([^\n]*).*$} $msg {\1}]" + } + # if no error (enough memory), or error by list creation - add it as skipped test: + if {![catch { + chan configure $pipe -buffering line + puts $pipe { + # test + set l [lrepeat 4000000 ""] + puts [llength [lsort $l]] + exit + } + set result [read $pipe] + close $pipe + set pipe {} + set result + } result] || [regexp {^(?:list creation failed|unable to alloc)} $result]} { + tcltest::Skip "prlimit: too small or too large AS-limit, result: $result" + } + set result + # expecting error no memory by sort +} -cleanup { + if {$pipe ne ""} { catch { close $pipe } } + unset -nocomplain pipe line result +} -result {no enough memory to proccess sort of 4000000 items} # Compiled version test cmdIL-6.1 {lassign command syntax} -body { -- cgit v0.12 From a3728e6d82a95897084c458b48661f468c14e724 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 17 Jan 2020 16:40:37 +0000 Subject: small amend (skip messages, avoid output on interactive shell) --- tests/cmdIL.test | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/cmdIL.test b/tests/cmdIL.test index b743b35..fabedea 100644 --- a/tests/cmdIL.test +++ b/tests/cmdIL.test @@ -454,14 +454,15 @@ test cmdIL-5.7 {lsort memory exhaustion} -constraints {prlimit} -body { exec prlimit -p [pid $pipe] --as=80000000 } msg]} { catch {close $pipe} - tcltest::Skip "prlimit: set of limit is impossible, [regsub {^\s*([^\n]*).*$} $msg {\1}]" + tcltest::Skip "prlimit: error - [regsub {^\s*([^\n]*).*$} $msg {\1}]" } # if no error (enough memory), or error by list creation - add it as skipped test: if {![catch { chan configure $pipe -buffering line puts $pipe { - # test - set l [lrepeat 4000000 ""] + # create list and get length (avoid too long output in interactive shells): + llength [set l [lrepeat 4000000 ""]] + # test OOM: puts [llength [lsort $l]] exit } @@ -470,7 +471,7 @@ test cmdIL-5.7 {lsort memory exhaustion} -constraints {prlimit} -body { set pipe {} set result } result] || [regexp {^(?:list creation failed|unable to alloc)} $result]} { - tcltest::Skip "prlimit: too small or too large AS-limit, result: $result" + tcltest::Skip "prlimit: wrong AS-limit, result: $result" } set result # expecting error no memory by sort -- cgit v0.12