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_INVALID,DKIM_SIGNED autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 23534 invoked from network); 6 Nov 2021 00:26:34 -0000 Received: from 4ess.inri.net (216.126.196.42) by inbox.vuxu.org with ESMTPUTF8; 6 Nov 2021 00:26:34 -0000 Received: from mail.9lab.org ([168.119.8.41]) by 4ess; Fri Nov 5 20:19:14 -0400 2021 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9lab.org; s=20210803; t=1636157151; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type; bh=KUch3CHvDBTqnU0FmoApO7BZcrTQoYKqS3gkzUrP1kY=; b=sBtWPqC7yGfBHMDoueuiWQJdGSs/fHYwAoobugZDVGdh8w2TArpcbt6w1K7hFzl1Q8QaJ6 cE8Oxvjl+KIQkXAOm/e80qNdM05kEg0LTuiIncH3NaZJ3wqZCR+V6EFdIfGW4q/mfYqAfQ CfWxUvV3MTgCleNEVYSrK1NWzwtZXSg= Received: from ken.9lab.home (host-185-64-155-70.ecsnet.at [185.64.155.70]) by mail.9lab.org (OpenSMTPD) with ESMTPSA id 602600e3 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Sat, 6 Nov 2021 01:05:51 +0100 (CET) Message-ID: To: 9front@9front.org CC: igor@9lab.org Date: Sat, 06 Nov 2021 01:05:50 +0100 From: igor@9lab.org MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="upas-ebpknuveygcwwnpatnuwdkffwd" List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: ISO-certified JSON app deep-learning-oriented generator Subject: [9front] acme: fix leaking memory allocated by getenv("font") Reply-To: 9front@9front.org Precedence: bulk This is a multi-part message in MIME format. --upas-ebpknuveygcwwnpatnuwdkffwd Content-Disposition: inline Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit The attached patch fixes a memory leak that occurs on a code path where the acme font is retrieved via `getenv("font")`. It is a small leak. Its presence is still annoying as it pollutes leak(1) output for acme. I am posting the patch here first in case there is review feedback. If no one objects I will push it over the weekend. Cheers, Igor From: Igor Böhm Date: Fri, 05 Nov 2021 23:51:55 +0000 Subject: [PATCH] acme: fix leaking memory allocated by getenv("font") If the font chosen for acme is retrieved via `getenv("font")` its memory is leaked: if(fontnames[0] == nil) fontnames[0] = getenv("font"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > getenv(…) mallocs memory if(fontnames[0] == nil) fontnames[0] = "/lib/font/bit/vga/unicode.font"; if(access(fontnames[0], 0) < 0){ fprint(2, "acme: can't access %s: %r\n", fontnames[0]); exits("font open"); } if(fontnames[1] == nil) fontnames[1] = fontnames[0]; fontnames[0] = estrdup(fontnames[0]); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > if the `getenv("font")` path was taken above, this assignment > will leak its memory. The following leak/acid session demonstrates the issue: cpu% leak -s 212252 src(0x002000cb); // 1 cpu% acid 212252 /proc/212252/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: src(0x002000cb) /sys/src/cmd/acme/acme.c:107 102 fprint(2, "usage: acme [-aib] [-c ncol] [-f font] [-F fixedfont] [-l loadfile | file...]\n"); 103 exits("usage"); 104 }ARGEND 105 106 if(fontnames[0] == nil) >107 fontnames[0] = getenv("font"); 108 if(fontnames[0] == nil) 109 fontnames[0] = "/lib/font/bit/vga/unicode.font"; 110 if(access(fontnames[0], 0) < 0){ 111 fprint(2, "acme: can't access %s: %r\n", fontnames[0]); 112 exits("font open"); acid: The fix tries to first check if a font has been set via command line options in which case the font string is malloced via estrdup(…). If no font has been selected on the command line getenv("font") is used. If no getenv("font") var is found we malloc a default font via estrdup(…). if(fontnames[0] != nil) fontnames[0] = estrdup(fontnames[0]); else if((fontnames[0] = getenv("font")) == nil) fontnames[0] = estrdup("/lib/font/bit/vga/unicode.font"); if(access(fontnames[0], 0) < 0){ fprint(2, "acme: can't access %s: %r\n", fontnames[0]); exits("font open"); } if(fontnames[1] == nil) fontnames[1] = fontnames[0]; fontnames[1] = estrdup(fontnames[1]); This resolves the memory leak and indeed leak(1) will not report any leaks ☺ --- diff 775608db7c004aca265474adb3e081f676b3b12a cb2f16fd1b18ff6bf12b594d129b8ff54423cb58 --- a/sys/src/cmd/acme/acme.c Fri Nov 5 00:11:56 2021 +++ b/sys/src/cmd/acme/acme.c Sat Nov 6 00:51:55 2021 @@ -103,17 +103,17 @@ exits("usage"); }ARGEND - if(fontnames[0] == nil) - fontnames[0] = getenv("font"); - if(fontnames[0] == nil) - fontnames[0] = "/lib/font/bit/vga/unicode.font"; + if(fontnames[0] != nil) + fontnames[0] = estrdup(fontnames[0]); + else + if((fontnames[0] = getenv("font")) == nil) + fontnames[0] = estrdup("/lib/font/bit/vga/unicode.font"); if(access(fontnames[0], 0) < 0){ fprint(2, "acme: can't access %s: %r\n", fontnames[0]); exits("font open"); } if(fontnames[1] == nil) fontnames[1] = fontnames[0]; - fontnames[0] = estrdup(fontnames[0]); fontnames[1] = estrdup(fontnames[1]); quotefmtinstall(); --upas-ebpknuveygcwwnpatnuwdkffwd Content-Disposition: attachment; filename=acme.getenv.font.patch Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit From: Igor Böhm Date: Fri, 05 Nov 2021 23:51:55 +0000 Subject: [PATCH] acme: fix leaking memory allocated by getenv("font") If the font chosen for acme is retrieved via `getenv("font")` its memory is leaked: if(fontnames[0] == nil) fontnames[0] = getenv("font"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > getenv(…) mallocs memory if(fontnames[0] == nil) fontnames[0] = "/lib/font/bit/vga/unicode.font"; if(access(fontnames[0], 0) < 0){ fprint(2, "acme: can't access %s: %r\n", fontnames[0]); exits("font open"); } if(fontnames[1] == nil) fontnames[1] = fontnames[0]; fontnames[0] = estrdup(fontnames[0]); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > if the `getenv("font")` path was taken above, this assignment > will leak its memory. The following leak/acid session demonstrates the issue: cpu% leak -s 212252 src(0x002000cb); // 1 cpu% acid 212252 /proc/212252/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: src(0x002000cb) /sys/src/cmd/acme/acme.c:107 102 fprint(2, "usage: acme [-aib] [-c ncol] [-f font] [-F fixedfont] [-l loadfile | file...]\n"); 103 exits("usage"); 104 }ARGEND 105 106 if(fontnames[0] == nil) >107 fontnames[0] = getenv("font"); 108 if(fontnames[0] == nil) 109 fontnames[0] = "/lib/font/bit/vga/unicode.font"; 110 if(access(fontnames[0], 0) < 0){ 111 fprint(2, "acme: can't access %s: %r\n", fontnames[0]); 112 exits("font open"); acid: The fix tries to first check if a font has been set via command line options in which case the font string is malloced via estrdup(…). If no font has been selected on the command line getenv("font") is used. If no getenv("font") var is found we malloc a default font via estrdup(…). if(fontnames[0] != nil) fontnames[0] = estrdup(fontnames[0]); else if((fontnames[0] = getenv("font")) == nil) fontnames[0] = estrdup("/lib/font/bit/vga/unicode.font"); if(access(fontnames[0], 0) < 0){ fprint(2, "acme: can't access %s: %r\n", fontnames[0]); exits("font open"); } if(fontnames[1] == nil) fontnames[1] = fontnames[0]; fontnames[1] = estrdup(fontnames[1]); This resolves the memory leak and indeed leak(1) will not report any leaks ☺ --- diff 775608db7c004aca265474adb3e081f676b3b12a cb2f16fd1b18ff6bf12b594d129b8ff54423cb58 --- a/sys/src/cmd/acme/acme.c Fri Nov 5 00:11:56 2021 +++ b/sys/src/cmd/acme/acme.c Sat Nov 6 00:51:55 2021 @@ -103,17 +103,17 @@ exits("usage"); }ARGEND - if(fontnames[0] == nil) - fontnames[0] = getenv("font"); - if(fontnames[0] == nil) - fontnames[0] = "/lib/font/bit/vga/unicode.font"; + if(fontnames[0] != nil) + fontnames[0] = estrdup(fontnames[0]); + else + if((fontnames[0] = getenv("font")) == nil) + fontnames[0] = estrdup("/lib/font/bit/vga/unicode.font"); if(access(fontnames[0], 0) < 0){ fprint(2, "acme: can't access %s: %r\n", fontnames[0]); exits("font open"); } if(fontnames[1] == nil) fontnames[1] = fontnames[0]; - fontnames[0] = estrdup(fontnames[0]); fontnames[1] = estrdup(fontnames[1]); quotefmtinstall(); --upas-ebpknuveygcwwnpatnuwdkffwd--