From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 10589 invoked from network); 2 Feb 2021 03:47:31 -0000 Received: from 1ess.inri.net (216.126.196.35) by inbox.vuxu.org with ESMTPUTF8; 2 Feb 2021 03:47:31 -0000 Received: from mail-wr1-f48.google.com ([209.85.221.48]) by 1ess; Mon Feb 1 18:09:47 -0500 2021 Received: by mail-wr1-f48.google.com with SMTP id c4so15720709wru.9 for <9front@9front.org>; Mon, 01 Feb 2021 15:09:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:message-id:to:cc:subject:date:mime-version :content-transfer-encoding; bh=FxjYQdJ4hKYQ1jyceD9i6Dvj5J3mjldu3IJWo7DqOMU=; b=BQAyhR/an73+Gn/kz21uzG+5nuNjSkANcEV7H+g9Elbb+jFqB6hXmyJQEz2f0l2T89 B+rzr731T6sX98MxSrPoZgTXnUwOhkrGK7UYWQZPUY1usqi9MHnt2nkKTN+ZPa7SbeV+ F7yrAWjjGRK31NbHkwz2KX6sSx8zdljT+MU7JtJKGSvICBW4av8HJmVXgb1rmtQbIyiw n3e2k260+62uW02Qys4B2fN74rGpevIPiC8jTSl2AEQ9ytMQtGf9R18EFgx8Ym4/foCh LICUgirtkwSkkmyHIC34ABFIpj8lTtA3jYu44cNIf8Sp7JXVbpt4Pu2LBCXboHtxE7BZ XCdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:to:cc:subject:date:mime-version :content-transfer-encoding; bh=FxjYQdJ4hKYQ1jyceD9i6Dvj5J3mjldu3IJWo7DqOMU=; b=rocFvUc88NDgLSE0CWobJGpnb8xq5LDVXfTaxq/MhBOi72ukywZdR2KvlQGe+3z2TN LE5CN1u9vVxTsfreQLisri/EMLSB/v9/UAaNKrpWEr4U3amfyH2BvUF7kz0uG7If40Np VkU4ae7wjhMbTpn4Jtr7vRFQ+c9VFfi7C/izZnhYLUpswsVjkNIgBwFDmIs0wgUgGFgE tUu8UaaD0R6Zw+1RftDCVRU42l8pnvmTeEoZ9q2Mj1CoZt5VCYhOFYbwSN16XVy57/xj OSGaaiHs1getS4ljnfhtRPPrhOoE8Jj1ksXw+OqG18tzcePBSdUYYZ4uhIDL4RDDDzU4 O2jw== X-Gm-Message-State: AOAM530RS0gPVCQcqpLhMEgotiXfaacqWUfUd8NiKU2cM+VSqoulZ/Gt ll4DnI1OY7CC9NJwQabmgcQ= X-Google-Smtp-Source: ABdhPJwzsTIHVk2n47VHKIUU0rK3gur95+FwlWUBdjQ7jGgGNzujsnZu+EKAQa2nvl3qha+1XU8Zgw== X-Received: by 2002:adf:efc2:: with SMTP id i2mr20058769wrp.422.1612220978518; Mon, 01 Feb 2021 15:09:38 -0800 (PST) Return-Path: Received: from term.home ([185.64.155.70]) by smtp.gmail.com with ESMTPSA id y83sm444091wmc.12.2021.02.01.15.09.37 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 01 Feb 2021 15:09:37 -0800 (PST) From: boehm.igor@gmail.com X-Google-Original-From: igor@gmail.com Message-ID: <863CA88C234EA794D5D1804F1D51D318@gmail.com> To: 9front@9front.org CC: boehm.igor@gmail.com Date: Tue, 02 Feb 2021 00:09:39 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: shared leveraged deep-learning XMPP session-aware callback generator Subject: [9front] cmd/acme: fix resource leak in ecmd.c (patch) Reply-To: 9front@9front.org Precedence: bulk There is a resource leak in acme functions: • /sys/src/cmd/acme/ecmd.c:/^B_cmd • /sys/src/cmd/acme/ecmd.c:/^D_cmd I will describe the case for function B_cmd, the issue is the same for D_cmd. In short, there can be a case where the heap allocated storage returned by the /sys/src/cmd/acme/ecmd.c:/^filelist(...) function is not freed. You can use the following as a starting point: • /sys/src/cmd/acme/ecmd.c:192 int B_cmd(Text *t, Cmd *cp) { Rune *list, *r, *s; int nr; list = filelist(t, cp->text->r, cp->text->n); ^^^^^^^^^^^^^^^ if(list == nil) editerror(Enoname); r = list; ... ... } clearcollection(); ^^^^^^^^^^^^^^^^^ return TRUE; } Notice the two lines that are highlighted, namely the call to filelist(...) and clearcollection(...). Now if we look at the impl of filelist(...) we have: /* string is known to be NUL-terminated */ Rune* filelist(Text *t, Rune *r, int nr) { if(nr == 0) return nil; r = skipbl(r, nr, &nr); if(r[0] != '<') return runestrdup(r); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ WHEN WE TAKE THE ABOVE PATH WE LEAK WHAT runestrdup(r) RETURNS /* use < command to collect text */ clearcollection(); runpipe(t, '<', r+1, nr-1, Collecting); return collection; ^^^^^^^^^^^^^^^^^^^^^^ TAKING THIS PATH IS FINE AS clearcollection(...) WILL FREE ANY HEAP ALLOCATED MEMORY } So for the case where we *do not* return the 'collection' pointer we leak memory that is allocated by 'runestrdup(...)'. There are probably more elegant solutions for this. The below simply checks if the pointer returned by filelist(...) is not equal to 'collection'. That is used as a sentinel to detect the case where runestrdup(...) has been called. That is the case where the memory returned by filelist(...) is *not* freed by clearcollection(...) and requires an extra free. Somehow the explanation is much more complicated than I initially intended. Hope you can still follow the reasoning and I haven't screwed up somewhere. Here is the patch: diff -r 0b8c8ef6a3d4 sys/src/cmd/acme/ecmd.c --- a/sys/src/cmd/acme/ecmd.c Tue Jan 19 15:18:57 2021 -0800 +++ b/sys/src/cmd/acme/ecmd.c Mon Feb 01 22:24:56 2021 +0100 @@ -204,6 +204,8 @@ if(nr > 0) r = skipbl(s+1, nr-1, &nr); } + if(list != collection) + free(list); clearcollection(); return TRUE; } @@ -278,6 +280,8 @@ if(nr > 0) r = skipbl(s+1, nr-1, &nr); }while(nr > 0); + if(list != collection) + free(list); clearcollection(); free(dir.r); return TRUE;