From 9b674e25817cdda29fdb393f9fcadc34517e179e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Dec 2018 11:27:40 +0100 Subject: [PATCH 1/6] core/device: fix typo --- src/core/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/device.c b/src/core/device.c index 5acd9b7a70..f8b6961d3c 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -821,7 +821,7 @@ static void device_enumerate(Manager *m) { r = sd_device_enumerator_new(&e); if (r < 0) { - log_error_errno(r, "Failed to alloacte device enumerator: %m"); + log_error_errno(r, "Failed to allocate device enumerator: %m"); goto fail; } From 296acffe45a74a8c9cca393494760169b5a4afd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 7 Dec 2018 16:12:19 +0100 Subject: [PATCH 2/6] When parsing paths, reject anything above PATH_MAX The check for length is done after path_simplify(), to be nice to paths which are constructed using specifiers, and have duplicate slashes and stuff. --- src/basic/path-util.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 995f39f16e..221517303e 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1139,5 +1139,12 @@ int path_simplify_and_warn( return -EINVAL; } + if (!path_is_valid(path)) { + log_syntax(unit, LOG_ERR, filename, line, 0, + "%s= path has invalid length (%zu bytes)%s.", + lvalue, strlen(path), fatal ? "" : ", ignoring"); + return -EINVAL; + } + return 0; } From f8703ed7e51121cd42bfb05fe9fe2a626118b07b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 7 Dec 2018 16:16:39 +0100 Subject: [PATCH 3/6] basic/path-util: line-break PATH_FOREACH_PREFIX macros Now I can see what they do :] --- src/basic/path-util.h | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 78db41b855..094aa47c01 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -97,12 +97,24 @@ int mkfs_exists(const char *fstype); /* Iterates through the path prefixes of the specified path, going up * the tree, to root. Also returns "" (and not "/"!) for the root * directory. Excludes the specified directory itself */ -#define PATH_FOREACH_PREFIX(prefix, path) \ - for (char *_slash = ({ path_simplify(strcpy(prefix, path), false); streq(prefix, "/") ? NULL : strrchr(prefix, '/'); }); _slash && ((*_slash = 0), true); _slash = strrchr((prefix), '/')) +#define PATH_FOREACH_PREFIX(prefix, path) \ + for (char *_slash = ({ \ + path_simplify(strcpy(prefix, path), false); \ + streq(prefix, "/") ? NULL : strrchr(prefix, '/'); \ + }); \ + _slash && ((*_slash = 0), true); \ + _slash = strrchr((prefix), '/')) /* Same as PATH_FOREACH_PREFIX but also includes the specified path itself */ -#define PATH_FOREACH_PREFIX_MORE(prefix, path) \ - for (char *_slash = ({ path_simplify(strcpy(prefix, path), false); if (streq(prefix, "/")) prefix[0] = 0; strrchr(prefix, 0); }); _slash && ((*_slash = 0), true); _slash = strrchr((prefix), '/')) +#define PATH_FOREACH_PREFIX_MORE(prefix, path) \ + for (char *_slash = ({ \ + path_simplify(strcpy(prefix, path), false); \ + if (streq(prefix, "/")) \ + prefix[0] = 0; \ + strrchr(prefix, 0); \ + }); \ + _slash && ((*_slash = 0), true); \ + _slash = strrchr((prefix), '/')) char *prefix_root(const char *root, const char *path); From 60473f0c23674bbde9f1b3cf1a2222a89c254886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 7 Dec 2018 16:22:10 +0100 Subject: [PATCH 4/6] pid1: fix (harmless) off-by-one in PATH_MAX comparison PATH_MAX is supposed to include the terminating NUL byte. But we already check that there is no NUL byte in the specified path. Hence the maximum length we can expect is PATH_MAX - 1. This doesn't change much, but makes this use of PATH_MAX consistent with the rest of the codebase. --- src/core/manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/manager.c b/src/core/manager.c index 871047e628..2398dcf4ea 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2222,7 +2222,7 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) { static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) { Manager *m = userdata; - char buf[PATH_MAX+1]; + char buf[PATH_MAX]; ssize_t n; n = recv(fd, buf, sizeof(buf), 0); From 4cb06c5949eba752f87dd6ad74620cc0e5cabf4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 7 Dec 2018 16:38:03 +0100 Subject: [PATCH 5/6] Use VLA instead of alloca The test is the same, but an array is more readable. --- src/core/unit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index e1b6e9f11c..a9303a0f3f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4580,7 +4580,6 @@ int unit_kill_context( int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) { _cleanup_free_ char *p = NULL; - char *prefix; UnitDependencyInfo di; int r; @@ -4620,7 +4619,7 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) return r; p = NULL; - prefix = alloca(strlen(path) + 1); + char prefix[strlen(path) + 1]; PATH_FOREACH_PREFIX_MORE(prefix, path) { Set *x; From a5dfc36ce60bf57ea319756a07a4a8a9f2a58b69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 7 Dec 2018 16:49:20 +0100 Subject: [PATCH 6/6] fuzz-unit-file: add one more test case There seems to be no error per se. RequiresMountsFor=%s%s%s..%s%s%s is expanded to RequiresMountsFor=/bin/zsh/bin/zsh/bin/zsh/bin/zsh/..., which takes a bit of time, and then we iterate over this a few times, creating a hashmap with a hashmap for each prefix of the path, each with one item pointing back to the original unit. Takes about 0.8 s on my machine. --- test/fuzz/fuzz-unit-file/oss-fuzz-11569 | Bin 0 -> 277466 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/fuzz/fuzz-unit-file/oss-fuzz-11569 diff --git a/test/fuzz/fuzz-unit-file/oss-fuzz-11569 b/test/fuzz/fuzz-unit-file/oss-fuzz-11569 new file mode 100644 index 0000000000000000000000000000000000000000..c49b8c6b1c5f671f4f1ce14adb0412817e5d685d GIT binary patch literal 277466 zcmeI*&2Agl6$kJv8v(QH1GuP}T@q20ZKZ&KriBByKpn$!SBX)mG*Jk3=v0)Ax_OoK z5xVWR`>y&3d6dqFNa|yFNB8n;hW>>yMe@#!W)9E&f1GnkO-ASM&W5ACd-v{rb5cEi zxp(q9f9d#M?@8~;r~Si&qr-lXOm)+y_1X6@vGDGSHBy7|Mud= z=oFHh>qac=DL z!<9o7M>@|>^w+)oI?7$<`%O1n^fTWw=3H(Zc(~}W#p?|R&d>PDMcz2@{OR>aC$j_p z^7gOJf8q2tkKXi^x^T}npKsK3*2R|dgU=689$|i04OGv|`{ISsq(0$_e7km=XI(!3 z%8`ax7Te6v>XX^gW)EhE|HbT4F_s6V$n^66>)S3J%gJPvAMyG@%U?M^f%zHFU+ewj z_Ab|7xl<^fk4|5GJ%0UucHr`TEiQI`D;}of!0b3h9+$V4UGcSRou1+RT=Vz*`WuvA zL-l@rH{Iw;O3hv>uJk;=yMKB3UzL|$J}DnnPpgOd{f*x-J3*LFVNc(=s}vgA_v@s zyX(i@#m=g%7k+!*8{aQVb02{Hx+DIJOKnZIsxix#o3(0jSpXg8k$H4$vIHu9@@4J$ zGA+1Yw%NL^K;_HY^W{ccR=zBtx&W%Rf5<#CkMQ&z&0%0Vn@8r6d1M|{OW0PgmEa3} zfiLg{zEo?7H|53PF5HE?a2M{v-7PJkxwcROAK@c>gpcqMKEg*i8@9AdiKp>&Ol}tW zQnZ)0na*-n&c-BQW4=@r-MCqi_kq^xt`3Qd>9*48W*+${!*j znQkkcZs;7Hqw|V%0v-_VCgLtSN9Xa&s7$xS>4whHIXXw@A<|h!=AiRXWYHGd;?Av` zvTmy4iFn~3;HJP3+)cz?bdJvBC#mU{INi8&?wmX4&O@ZLjLbpjp~#{ww8fos=iE7W z-ic2&p>uT3ov-TSO5`8;NB)t2Ta$n29G#`Hw%pr3&Aal?;I!EW| z9G!3DZ~c%tWX?)u4mwBY=p3D+b97#ci}nBEBqB7jA$6a)e&bf2$oIB^v z+v9l2zeM?m&e1tKN9X7qoul(sEF4&-%7xC+IXXw@=p3D+b97#^VL(uHdPvbHvFM|> zRo~kZ7jd!H!di=vhYT=gBOh_+p~yCd4?Kfs@C=?qAhHZR!x#7xgD>bjG!xM|cWy=Q z2ENS-ZK16g+LABz`GU^Tx!$?nx!$?nd1+o+(LQFQ5^WSZN9X7qouhMfj?P=5WtM5q zpmTJN&e1tKN9X7qotJDF5EPvrQuIkI`si)d_qN1ET=a-54;f$vxSNQ(+&Op7opa~h zId{&TbLZT7KxOXg5cQm>p1E`GoIB^vLmo0f8jZNiorfaZ7(Va}p20JC4uQxr@C;wz zOANlC^UzF0=Uk}Io%`JRuu-rCI6E)|U*OA1@dcfubG>uDbG>uD^HN;2qJ2!}B+49g zj?U3JI!EW|9G$m9%PdpPLFeckouhMfj?U3JIxpEUASgOLr0A1a^eNu;PQUYMCof;* z%SwBp=8>gvmcnsc+!nXh9!W*!kU3<|29}g6wuf9K7y?6JXag|Bg>s=WE>ynM=L4sX52My|5M+aj|sL(#fqYo#f8BbMBlw z=gzrv?wmX4&Ue*<2d;|?- z384w0384uQ@=5__fV+vf%bkZZPFf|V6*@=f=p3Dgz!ja>gX=PJLFb{2L+AAwCobY* zY10OlHj%|-af~cR=jdGTT<=`(T<^RT>8)rVvkQrK0iC0BbdJu^IXXw@tS zl4bG49LSx(5cQm>o)u3OPh*Ow@oWv$4I!EW|9G#z(VJ>z(VJmt@h3_HiR^r8DAgS98!gI!EW|9G#;T0@+Br;xO48D zJLk^1bMBlww-l~q!$L&HlS2FV;?B8qy>q>Dz4JAcJ_GO%cN1}!JLk^hg_!A-c(|$_~dHq5!xCh8l5~JLk^hld&+U?j?NX86_pj06_wjlIU@hazm>{A z?wmU(bI2Srhs+^!^v?CpceN25xGt`Ot5~V4P&}1-q}C&Q;vz0&4w*ydY**&oO|=AQ z5s(A&7$BD~@zQC8R;LG#T>8r2DuitOukFo`jB5g=NpdNa}MOQz?U}ROU%0wnAj~Df-mqTCZpwx ze338sNPSm*SA92UL+cr=XQ-f{C5k@nH0V0IE(Xiwi+qtUe855<&>P=>_T`HwKh;l@ zG?MD{UJ$cB6~skcFbO7E$RtZHpn)&&1-`%+_@ZL1Vx6<0inWThiuJJ5#e~4g)4TL; z493eB`66FZ{n1MPN}DNY3YvncQ}FTl&G_Q@-Y~z*KFa^gKHhsges^|0`fjHNdXwp` zd@#SOUap>BZd3i(`H*z^0)Kn@{pp|2UY}jO|IM4X<5y?nznqLNj^FKP2S3aD53)&i znDw)V+0o@YTD{4iTz!@uJjnWo+2Any_;}pU_V=^?L3U9*?Dw<7pAWJJ*->`zMb>|m z4F=^lpJv(7!~K3%{y5mr1`o2q!^>Ury^pfu2>B7KJ&&?MzGv*##kuSc^2^vCJUTq+ zX9t5Uzo`9#tb8piUa3wuKc`1oes*7EgM+=xXp}^_;tz^`nB@A;AAkH`IGP`3T$-!E z;zEoOW65G{)~z*<7R{rVpZC5!w?68Ib(SxS@+B`-%a>b_EMMfyt)$QwsZWY^$7K@z bYXfK0k|6+7|E9JqE|bkJYY8N0QegfM)Gs$k literal 0 HcmV?d00001