Artifact [bb8c278bf2]

Artifact bb8c278bf244f123958c3f4901da2fe84976cd8da778b18f8aa54a39d86d649e:

Ticket change [bb8c278bf2] - Ticket [e01f02a12180ac37|e01f02a121] <i>Segfault under wish at first tls::import, not under tclsh</i> status still Open with 6 other changes by anonymous on 2020-09-01 09:44:20.
D 2020-09-01T09:44:20.849
J icomment Okay,\sI\sthink\sI've\sdiagnosed\sthis\snow,\sthough\sI\sdon't\sknow\show\sto\sfix\sit.\r\nThere\sdoesn't\sseem\sto\sbe\san\serror\sin\sthe\sTclTLS\ssource\scode\sas\ssuch,\sso\r\nthe\sfix\sis\sprobably\srather\sto\sadjust\sthe\sbuild\sprocess\s--\sfigure\sout\sthe\r\nright\soptions\sto\spass,\sand\sit\sshould\swork.\sHowever\sI\ssuspect\sTclTLS\sstill\s\r\nneeds\simproving\sbecause\sof\sthis\sissue,\ssince\sit\sdoes\sseem\sto\sbe\sa\smore\s\r\ngeneral\svulnerability;\sit\sshould\sbe\seasy\sto\smake\sa\shardened\sbuild,\seven\s\r\nif\sone\sdoes\snot\smake\sit\sthe\sdefault.\r\n\r\nIn\sshort,\sthe\sproblem\sis\sthat\sunder\sWish\sI'm\sgetting\stwo\sdifferent\s\r\nversions\sof\slibcrypto\sloaded,\sand\sthe\sdynamic\slinker\sis\smaking\sTclTLS\s\r\ncall\ssome\sfunctions\sfrom\sone\sversion\sand\sother\sfunctions\sfrom\sthe\sother;\s\r\nsince\sthey\sare\srather\sfar\sfrom\sbeing\sbinary\scompatible,\sa\scrash\sis\sonly\s\r\nto\sbe\sexpected.\stclsh\savoids\sthe\sproblem\sby\snot\sloading\sthe\sfirst\sversion\s\r\nof\slibcrypto,\sso\sthat\sall\scalls\sgo\sto\sthe\ssecond\sversion\sagainst\swhich\s\r\nTclTLS\swas\sactually\sbuilt,\s*but*:\sshould\stclsh\sload\sany\sother\slibrary\s\r\nwith\sa\sdependence\supon\sthe\sfirst\slibcrypto\sversion\sbefore\sit\sloads\sTclTLS\s\r\nthen\sthe\sproblem\swill\ssurface\salso\sthere.\sRight\snow\sTclTLS\sis\svulnerable\s\r\nin\sthat\sit\sdepends\son\sother\sparties\snot\sdoing\sthings\sit\shadn't\sexpected,\s\r\ninstead\sof\smaking\ssure\sthat\sit\sgets\swhat\sit\srequires.\r\n\r\nIt\smight\sbe\sargued\sthat\shaving\sdifferent\sversions\sof\sa\slibrary\sin\sthe\s\r\nsame\sprocess\sis\sa\sbig\sno-no,\sand\sthat\sthe\sfix\sis\sto\supdate\severything\sto\s\r\nuse\sthe\ssame\smost\srecent\sversion,\sbut\sthat\sis\sfrankly\simpossible;\sthe\s\r\nold\slibcrypto\sis\sbeing\sbrought\sin\sby\ssome\ssystem\slibrary\s(don't\sknow\s\r\nwhich\sone,\sbut\sthe\sone\snext\sto\sit\sin\sthe\slist\sis\scalled\sTrustEvaluationAgent,\r\nwhich\sseems\sa\splausible\suser)\sand\sthis\sain't\slinux\s--\swe\scan't\shope\sto\s\r\ngo\srecompiling\sthose.\sMoreover\sit\smight\sbe\sremarked\sthat\sthe\s(long)\slist\s\r\nof\sshared\slibraries\sWish\sloads\sincludes\sa\sdecade-old\slibsqlite3.dylib,\s\r\nbut\sthat\shas\snever\sbeen\sa\sproblem\sfor\stclsqlite3,\spresumably\sbecause\sthat\s\r\nlinks\sstatically\swith\sthe\ssqlite3\slibrary.\sI\ssee\sno\sreason\swhy\sTclTLS\s\r\ncouldn't\shave\san\soption\sto\sdo\sthe\ssame,\sbut\sI\scannot\sfind\sanything\sin\s\r\nthe\sdocumentation\sto\sthat\seffect.\r\n\r\nIndeed,\sthe\sdocumentation\sfor\sOpenSSL\s1.0.x\sstrongly\srecommends\slinking\s\r\nstatically\sagainst\sthe\slibrary\s(for\ssecurity\sreasons),\sthough\sthe\s\r\nOpenSSL\s1.1.x\sdocumentation\sdoes\snot\smake\sthis\spoint\s(that\sI\scan\ssee).\s\r\nI\shave\showever\sonly\sbeen\sable\sto\sbuild\sTclTLS\sto\slink\sagainst\sthe\sdylibs.\r\n\r\nMuch\slonger,\smy\sinvestigation\swas\sas\sfollows:\r\n\r\nI\ssaw\sthe\sprogram\scrash\sin\spthread_rwlock_wrlock,\scalled\sfrom\sthe\s\r\n\tSSL_CTX_set_tmp_dh(ctx,\sdh);\r\nline\sin\sCTX_Init\s(tls.c\saround\sline\s1260,\sbut\sI've\sdone\ssome\shacking\s\r\nto\sdebug\sso\smy\sline\snumbers\sare\soff),\swith\sa\s\r\n\s\sKERN_INVALID_ADDRESS\sat\s0x0000000000000000\r\nSo...\sa\sNULL\spointer\sdereferencing.\sIt\sturns\sout\sthe\spointer\sin\s\r\nquestion\sis\sdh->lock\sup\sin\stls.c\s(or\sit\swould\sbe,\shad\sthe\sDH\stype\snot\s\r\nbeen\sopaque\sin\stls.c,\sbut\sit\sis\sa\sfield\sin\sthe\sstruct\sthat\sdh\spoints\sto).\s\r\nWhat\sSSL_CTX_set_tmp_dh\sis\sdoing\sat\sthe\stime\sof\sthe\scrash\sis\sthat\sit\s\r\nis\sadjusting\sthe\srefcount\sof\sthose\sDH\sparameters,\sand\sthe\slock\sin\s\r\nquestion\sis\sprotecting\sthat\srefcount.\r\n\r\nThe\sCRYPTO_UP_REF\soperation\sbeing\sused\sdoes\shave\ssomething\slike\sfour\s\r\ndifferent\simplementations,\smany\sof\swhich\sdon't\sneed\sthis\slock\sat\sall\s\r\n(but\snot\sthe\slast\sresort\simplementation\svia\spthreads\sI\send\sup\swith),\s\r\nso\sit\swould\snot\sbe\soutrageous\sto\ssuspect\sa\sbug\sin\slibcrypto\swhere\sthis\s\r\nfield\snever\sgets\sinitialised,\sbut\sthat\sis\snot\swhat\sis\sgoing\son.\r\n\r\nSome\shacking\sto\sinsert\sstatements\sprinting\sthe\svalue\sof\sdh->lock\s\r\nreveals\sthat\sit\sis\sindeed\sNULL\swhen\srun\sunder\sWish,\sbut\snon-NULL\s\r\nwhen\srun\sunder\stclsh.\sSome\smore\shacking\sin\sdh_params.h\sreveals\sthat\s\r\nthis\sis\swhat\sthat\sstruct\sis\slike\salready\swhen\sDH_new\sreturns\sit.\s\r\nThis\scould\sstill\sbe\sa\smatter\sof\suninitialised\smemory\sjust\shappening\s\r\nto\shave\sa\sfortuitous\svalue\sin\stclsh\sbut\snot\sWish,\sthough\sto\sbe\ssure\s\r\none\sneeds\sto\ssee\swhat\sDH_new\s(or\srather\sthe\sunderlying\sDH_new_method,\s\r\nboth\sliving\sin\sopenssl-1.1.1g/crypto/dh/dh_lib.c)\sis\sdoing.\r\n\r\nThis\sis\swhere\sthings\sget\sweird,\sbecause\sfrom\sRTFS\sthat\sfunction\shas\s\r\nan\sexplicit\scheck\swhether\slock\sis\sNULL,\sand\swill\sreturn\sNULL\sfor\sthe\s\r\ndh\spointer\sif\sit\sis.\sYet\swhen\srun,\sDH_new\sis\sreturning\sa\snon-NULL\s\r\npointer\sfor\sdh\ssuch\sthat\sdh->lock\sis\sNULL.\sThis\sshould\snot\shappen.\s\r\nMight\ssome\slater\sinstruction\sin\sDH_new_method\sbe\soverwriting\sthe\slock\s\r\nwith\sgarbage?\sOnly\sway\sto\stell\sis\sto\sbring\sout\sa\sdebugger.\sThat's\s\r\nwhere\sthings\sget\s*really*\sweird.\r\n\r\n(In\sretrospect,\sworking\swith\sthis\sissue\sthe\sfirst\stime\sone\suses\sgdb\s\r\nis\sprobably\snot\sideal,\sbut\shey,\sI've\slearnt\ssome\sgdb\snow!\sGood\sfor\sme.)\r\n\r\nFor\ssome\sreason,\sgdb\srefuses\sto\sstep\sinto\sthe\sDH_new\sfunction,\s\r\ncomplaining\sabout\sthere\snot\sbeing\sline\snumber\sinformation\savailable.\s\r\nIn\sretrospect\sthis\scould\shave\sbeen\sa\sclue,\sbut\sI\sspend\sa\slot\sof\stime\s\r\nfiddling\swith\scompiler\soptions\sfor\sdebug\sinformation,\shoping\sthat\sthis\s\r\nwill\sdo\sthe\strick.\sIt\smight\seven\shave\shelped\sa\sbit,\sbut\seventually\sI'm\s\r\ndown\sto\sstepping\sat\sthe\smachine\scode\slevel.\sThe\sgdb\slist\scommand\sis\snot\s\r\nshowing\sanything\sthat\smakes\ssense,\sbut\sthe\sdisassemble\scommand\sis\smy\s\r\nfriend,\sonce\sI\srefresh\smyself\son\sthe\smemnonics.\sWhat\sI\ssee\sis\s…\s\r\nplausibly\sthe\sDH_new_method\sfunction,\sbut\sit\slooks\slike\sit\shas\sbeen\s\r\noptimised\sto\shell\sand\sback,\sand\sindeed\sthe\scrucial\sCRYPTO_THREAD_lock_new\s\r\ncall\sis\smissing.\sBut\sthis\slibcrypto\swas\scompiled\swith\s-O0,\sso\swhy\swould\s\r\nit\sbe\soptimised??\r\n\r\nIndeed,\swhen\srunning\sgdb\son\sjust\slibcrypto.1.1.dylib\sI\sget\sa\sdisassembly\s\r\nlooking\sthe\sway\sI\smight\sexpect\s(even\soverly\sroundabout).\sEven\smore\s\r\nstrange,\sI\sget\sthe\ssame\slisting\swhen\sI\sask\sthe\sgdb\sdebugging\sWish\sto\s\r\n\s\sdisassemble\sDH_new_method\r\nSo\swhy\sisn't\s*that*\sthe\scode\sthat\sthis\sprocess\sis\srunning??\sThen\sit\shits\s\r\nme:\sthe\saddresses\sdon't\smatch.\sThe\sgood\sDH_new_method\sis\sat\s\r\n0x0000000114e7bfe0,\sbut\sthe\sone\sthat\sgets\scalled\sis\sat\s0x00007fff8a4e6e90.\s\r\ngdb\ssays\sboth\sare\scalled\sDH_new_method,\sbut\s0x00007fff8a4e6e90\sis\sin\s\r\nlibcrypto.0.9.8.dylib,\swhereas\s0x0000000114e7bfe0\sis\sin\s\r\nlibcrypto.1.1.dylib,\sjust\slike\scall\sthat\sinvokes\sthe\sCRYPTO_UP_REF\s\r\noperation.\sMystery\ssolved,\sto\ssome\sextent.\r\n\r\nNow\sI\sonly\sneed\sto\sfigure\sout\show\sto\smake\sTclTLS\sbind\sto\sthe\sright\s\r\ninstances\sof\sthese\ssymbols.\sIt\swould\shelp\sgetting\ssome\sinput\sfrom\s\r\npeople\swho\sactually\sknow\show\sall\sthese\sthings\sare\ssupposed\sto\sfit\s\r\ntogether,\sinstead\sof\shaving\sto\sguess\swhile\srandomly\spoking.
J login anonymous
J mimetype text/x-fossil-plain
J priority Immediate
J resolution Open
J username lars_h
K e01f02a12180ac37cdb96ec16d0e03f89033cd77
U anonymous
Z bd1e2c49734b2cc24eed0964a2450bc3