Bug#1036175: unblock: mksh/59c-28
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: mksh@packages.debian.org, tg@mirbsd.de
Control: affects -1 + src:mksh
Please unblock package mksh
[ Reason ]
Some bugfixes regarding formatting, shift/rotate arithmetics,
and a few cases where the parser was not properly reentrant
leading to use-after-free/double-free for recursive parsing.
Also add some tests for the printf builtin to the testsuite.
[ Impact ]
Probably low. I haven’t hit any of these in practice, but
fuzzing would find them.
[ Tests ]
There is a new test for the printf builtin, but the other
changes have only been manually tested to not crash (also
with Valgrind and ASan/UBSan) after the change.
[ Risks ]
I’ve only cherry-picked the changes I’m fully confident of
and that do not touch larger areas of code. They have been
in use for several weeks, and no new issues popped up.
[ Checklist ]
[x] all changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[↓] attach debdiff against the package in testing
[ Other info ]
Instead of a debdiff, because all changes are contained in
d/patches/debian-changes, I attached a diff of the unpacked
and patched package minus d/patches/.
The changes to Build.sh and check.t are to test the printf builtin.
The change to mbsdint.h is the arithmetic fix: a % b instead of a & (b-1)
The change to shf.c is to fix the formatting stuff.
The changes to sh.h and syn.c are the parser fixes, plus sh.h
assigns a unique upstream revision date to the so-patched version
to ease $KSH_VERSION-based bugspotting.
I cherry-picked the fixes only but kept the RCS IDs as is in order
to not clutter the diff for easier review.
unblock mksh/59c-28
diff -pruN mksh-59c-26/Build.sh mksh-59c-28/Build.sh
--- mksh-59c-26/Build.sh 2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/Build.sh 2023-05-16 19:13:32.000000000 +0200
@@ -2805,6 +2805,7 @@ fi
if test 1 = "$USE_PRINTF_BUILTIN"; then
cpp_define MKSH_PRINTF_BUILTIN 1
+ check_categories="$check_categories printf-builtin"
else
USE_PRINTF_BUILTIN=0
fi
diff -pruN mksh-59c-26/check.t mksh-59c-28/check.t
--- mksh-59c-26/check.t 2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/check.t 2023-05-16 19:13:32.000000000 +0200
@@ -31,7 +31,7 @@
# (2013/12/02 20:39:44) http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/regress/bin/ksh/?sortby=date
expected-stdout:
- KSH R59 2023/01/31
+ KSH R59 2023/04/28
description:
Check base version of full shell
stdin:
@@ -14254,3 +14254,43 @@ expected-stdout:
2 eh .
3 eh .
---
+name: optional-printf-builtin
+description:
+ printf(1) minimal tests
+category: printf-builtin
+stdin:
+ t() {
+ o=$(\\builtin printf "$@"; echo .)
+ r=$?
+ print -r -- "$((++n)) ${o%.} : $r"
+ }
+ t '<%s>' a b\ c
+ t '%s - %c %s %d %i %o %u %x %X %b' 3
+ t '%% %c %s %d %i %o %u %x %X' 72 73 74 75 76 77 78 79
+ t '\\\a\b\f\n\r\t\v\01234'
+ let n+=2
+ t '%b foo %s bar' '\\\a\b\f\n\r\t\v\01234\cdefg'
+ let ++n
+ t '<%d>' 1 +2 -3 0x1a 0X1B 010 -011 +012 \'1 \"2
+ t '<%d|%d><%+d|%+d><% d|% d><%+ d|%+ d>' 1 -1 1 -1 1 -1 1 -1
+ t '<%o|%#o><%x|%#x><%X|%#X>' 8 9 10 11 12 13
+ #XXX replace with below
+ t '|%3d|%-3d|%+3d|%-+3d|%03d|%-03d|%3d|%.3d|notyet|' \
+ 1 2 3 4 5 6 7777 8
+ #t '|%3d|%-3d|%+3d|%-+3d|%03d|%-03d|%3d|%.3d|%5.3d|' \
+ # 1 2 3 4 5 6 7777 8 9
+ #12 | 1|2 | +3|+4 |005|6 |7777|008| 009| : 0
+expected-stdout:
+ 1 <a><b c> : 0
+ 2 3 - 0 0 0 0 0 0 : 0
+ 3 % 7 73 74 75 114 77 4e 4F : 0
+ 4 \
+
+ 34 : 0
+ 7 \
+
S4 : 0
+ 9 <1><2><-3><26><27><8><-9><10><49><50> : 0
+ 10 <1|-1><+1|-1>< 1|-1><+1|-1> : 0
+ 11 <10|011><a|0xb><C|0XD> : 0
+ 12 | 1|2 | +3|+4 |005|6 |7777|008|notyet| : 0
+---
diff -pruN mksh-59c-26/debian/changelog mksh-59c-28/debian/changelog
--- mksh-59c-26/debian/changelog 2023-02-15 16:04:53.000000000 +0100
+++ mksh-59c-28/debian/changelog 2023-04-28 23:34:20.000000000 +0200
@@ -1,3 +1,21 @@
+mksh (59c-28) unstable; urgency=medium
+
+ * Revert 59c-27 changes as mksh is, surprisingly, still a key
+ package for this release, shunit check-B-D
+ * Add cherry-picked individual fixes
+ - fix some formatting routine corner cases
+ - check the optional printf builtin (used for lksh) in the
+ testsuite, to avoid the issues reappearing
+ * Cherry-pick more individual fixes from upstream
+ - fix shift/rotate for nōn-power-of-two-sized bit quantities
+ - correct 59c regression in recursive parser for command
+ substitution and fix the other place it was not reentrant
+ as well (by moving function-static storage to stack/heap);
+ crash discovered by Riccardo Felici, USI Lugano
+ * Revert RCS ID changes, but assign individual version datestamp
+
+ -- Thorsten Glaser <tg@mirbsd.de> Fri, 28 Apr 2023 23:34:20 +0200
+
mksh (59c-26) unstable; urgency=medium
* Fixup more update-shells breakage also found by piuparts
diff -pruN mksh-59c-26/mbsdint.h mksh-59c-28/mbsdint.h
--- mksh-59c-26/mbsdint.h 2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/mbsdint.h 2023-05-16 19:13:32.000000000 +0200
@@ -675,7 +675,7 @@ mbiCTAS(mbsdint_h) {
#define mbiK_sr(ut,n,vl,vr,vz) mbiK_SR(ut, mbiTYPE_UBITS(ut), n, vl, vr, vz)
#define mbiMK_sr(ut,FM,n,l,r,z) mbiMM(ut, (FM), mbiK_SR(ut, mbiMASK_BITS(FM), \
n, mbiMM(ut, (FM), (l)), (r), (z)))
-#define mbiK_SR(ut,b,n,l,r,vz) mbiK_RS(ut, b, n, l, mbiUI(r) & (b - 1U), (vz))
+#define mbiK_SR(ut,b,n,l,r,vz) mbiK_RS(ut, b, n, l, mbiUI(r) % mbiUI(b), (vz))
#define mbiK_RS(ut,b,n,v,cl,zx) mbiOT(ut, cl, n(ut, v, cl, b - (cl), zx), v)
#define mbiK_shl(ut,ax,cl,CL,z) mbiOshl(ut, ax, cl)
#define mbiK_shr(ut,ax,cl,CL,z) mbiOshr(ut, ax, cl)
diff -pruN mksh-59c-26/sh.h mksh-59c-28/sh.h
--- mksh-59c-26/sh.h 2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/sh.h 2023-05-16 19:13:32.000000000 +0200
@@ -235,7 +235,7 @@
#define __SCCSID(x) __IDSTRING(sccsid,x)
#endif
-#define MKSH_VERSION "R59 2023/01/31"
+#define MKSH_VERSION "R59 2023/04/28"
/* shell types */
typedef unsigned char kby; /* byte */
@@ -2150,6 +2150,7 @@ struct ioword {
#define IOBASH BIT(10) /* &> etc. */
#define IOHERESTR BIT(11) /* <<< (here string) */
#define IONDELIM BIT(12) /* null delimiter (<<) */
+#define IOSYNIONEXT BIT(13) /* already fully configured */
/* execute/exchild flags */
#define XEXEC BIT(0) /* execute without forking */
diff -pruN mksh-59c-26/shf.c mksh-59c-28/shf.c
--- mksh-59c-26/shf.c 2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/shf.c 2023-05-16 19:13:32.000000000 +0200
@@ -837,20 +837,15 @@ shf_smprintf(const char *fmt, ...)
char *
kslfmt(ksl number, kui flags, char *numbuf)
{
+ /* easy for positive number */
+ if (number >= 0)
+ return (kulfmt((kul)number, flags, numbuf));
+ /* negative signed quantity */
if (!IS(flags, FM_TYPE, FL_SGN)) {
- /* uh-oh, unsigned? what? be bitwise faithful here */
- union {
- /*XXX hopefully not UB… */
- ksl s;
- kul u;
- } v;
-
- v.s = number;
- return (kulfmt(v.u, flags, numbuf));
+ /* uh-oh, output a signed quantity unsignedly */
+ return (kulfmt(KSL2UL(number), flags, numbuf));
}
- if (number < 0)
- flags |= FL_NEG;
- return (kulfmt(KSL2NEGUL(number), flags, numbuf));
+ return (kulfmt(KSL2NEGUL(number), flags | FL_NEG, numbuf));
}
/* pre-initio() */
@@ -892,13 +887,13 @@ kulfmt(kul number, kui flags, char *numb
number /= 10UL;
} while (number);
- if (!IS(flags, FM_TYPE, FL_DEC)) {
+ if (IS(flags, FM_TYPE, FL_SGN)) {
if (HAS(flags, FL_NEG))
*--cp = '-';
else if (HAS(flags, FL_PLUS))
*--cp = '+';
else if (HAS(flags, FL_BLANK))
- *--cp = '-';
+ *--cp = ' ';
}
break;
}
diff -pruN mksh-59c-26/syn.c mksh-59c-28/syn.c
--- mksh-59c-26/syn.c 2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/syn.c 2023-05-16 19:13:32.000000000 +0200
@@ -191,19 +191,16 @@ static struct ioword *
synio(int cf)
{
struct ioword *iop;
- static struct ioword *nextiop;
Wahr ishere;
- if (nextiop != NULL) {
- iop = nextiop;
- nextiop = NULL;
- return (iop);
- }
-
if (tpeek(cf) != REDIR)
return (NULL);
ACCEPT;
iop = yylval.iop;
+ if (iop->ioflag & IOSYNIONEXT) {
+ iop->ioflag &= ~IOSYNIONEXT;
+ return (iop);
+ }
ishere = (iop->ioflag & IOTYPE) == IOHERE;
if (iop->ioflag & IOHERESTR) {
musthave(LWORD, 0);
@@ -228,17 +225,21 @@ synio(int cf)
if (iop->ioflag & IOBASH) {
char *cp;
- nextiop = alloc(sizeof(*iop), ATEMP);
- nextiop->ioname = cp = alloc(3, ATEMP);
+ iop->ioflag &= ~IOBASH;
+
+ cp = alloc(sizeof(struct ioword) + 3U, ATEMP);
+ yylval.iop = (void *)cp;
+ cp += sizeof(struct ioword);
+ yylval.iop->ioname = cp;
*cp++ = CHAR;
*cp++ = digits_lc[iop->unit];
*cp = EOS;
-
- iop->ioflag &= ~IOBASH;
- nextiop->unit = 2;
- nextiop->ioflag = IODUP;
- nextiop->delim = NULL;
- nextiop->heredoc = NULL;
+ yylval.iop->delim = NULL;
+ yylval.iop->heredoc = NULL;
+ yylval.iop->ioflag = IODUP | IOSYNIONEXT;
+ yylval.iop->unit = 2;
+ REJECT;
+ symbol = REDIR;
}
return (iop);
}
@@ -282,7 +283,7 @@ get_command(int cf, int sALIAS)
XPtrV args, vars;
struct nesting_state old_nesting;
Wahr check_decl_utility;
- static struct ioword *iops[NUFILE + 1];
+ struct ioword *iops[NUFILE + 1];
XPinit(args, 16);
XPinit(vars, 16);
Reply to: