RootDirectory=, ProtectKernelTunables=yes cause "ERROR: AddressSanitizer: stack-buffer-overflow" #4567
Comments
This was referenced Nov 5, 2016
|
So, actually this index db9a7aa..ab136b0 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -196,7 +196,7 @@ static int append_target_mounts(BindMount **p, const char *root_directory, const
* declaration we do not support "-" at the beginning.
*/
const TargetMount *m = &mounts[i];
- const char *path = prefix_roota(root_directory, m->path);
+ const char *path = prefix_root(root_directory, m->path);
if (!path_is_absolute(path))
return -EINVAL;fixes the stack-buffer-overflow (but introduces a memory leak, of course) cc @tixxdz |
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 6, 2016
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= works with ProtectKernelTunables=, ProtectControlGroups= and ProtectKernelModules=yes Fixes: systemd#4567
I'm not sure. But seems like this never works (I checked there: #4588). This is why the |
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 6, 2016
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= work with ProtectHome=, ProtectControlGroups= and ProtectKernelModules=yes Fixes: systemd#4567
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 6, 2016
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= work with ProtectHome= and ProtectKernelModules=yes Fixes: systemd#4567
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 6, 2016
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= work with ProtectHome= and ProtectKernelModules=yes Fixes: systemd#4567
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 6, 2016
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= work with ProtectHome= and ProtectKernelModules=yes Fixes: systemd#4567
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 7, 2016
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= work with ProtectHome= and ProtectKernelModules=yes Fixes: systemd#4567
|
Something went wrong #4594 (comment) |
|
I don't know how to reproduce. But I modified the diff --git a/stage1/usr_from_src/usr_from_src.mk b/stage1/usr_from_src/usr_from_src.mk
index 9aa259c..408db88 100644
--- a/stage1/usr_from_src/usr_from_src.mk
+++ b/stage1/usr_from_src/usr_from_src.mk
@@ -99,7 +99,7 @@ $(call generate-stamp-rule,$(UFS_ROOTFS_STAMP),$(UFS_SYSTEMD_INSTALL_STAMP),$(S1
# this installs systemd into temporary rootfs
$(call generate-stamp-rule,$(UFS_SYSTEMD_INSTALL_STAMP),$(UFS_SYSTEMD_BUILD_STAMP),$(UFS_ROOTFSDIR), \
$(call vb,v2,INSTALL,systemd) \
- DESTDIR="$(abspath $(UFS_ROOTFSDIR))" $$(MAKE) -C "$(UFS_SYSTEMD_BUILDDIR)" V=0 install-strip $(call vl2,>/dev/null))
+ DESTDIR="$(abspath $(UFS_ROOTFSDIR))" $$(MAKE) -C "$(UFS_SYSTEMD_BUILDDIR)" V=0 install $(call vl2,>/dev/null))Backtrace: Line 917 in 453a9c7 finish:
for (m = mounts; m < mounts + n_mounts; m++)
free(m->path);
return r; |
|
I've installed |
|
@evverx ok will make some time today, and check thanks |
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 8, 2016
…s() fails This certainly fixes a bug that was introduced by PR systemd#4594 that intended to fix systemd#4567. The fix was not complete. This patch makes sure that we count freed paths when chase_all_symlinks() fails and we do not return directly. Also go ahead and remove assert() checks against mount counts being zero. This does not make sense since we are explicitly decrementing and freeing mount paths which makes it easy to hit the assert. If users did provide a misconfigured paths then just log a debug message, ignore and continue. Note that if mount count is zero at the beginning, then we we never hit this code path. Fixes systemd#4567
|
btw it seems we didn't lookup on how to hookup rkt tests at least current rkt master or the ones that make sense for systemd |
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 8, 2016
…s() fails This certainly fixes a bug that was introduced by PR systemd#4594 that intended to fix systemd#4567. The fix was not complete. This patch makes sure that we count freed paths when chase_all_symlinks() fails and we do not return directly. Also go ahead and remove assert() checks against mount counts being zero. This does not make sense since we are explicitly decrementing and freeing mount paths which makes it easy to hit the assert. If users did provide a misconfigured paths then just log a debug message, ignore and continue. Note that if mount count is zero at the beginning, then we we never hit this code path. Fixes systemd#4567
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 8, 2016
…s() fails This certainly fixes a bug that was introduced by PR systemd#4594 that intended to fix systemd#4567. The fix was not complete. This patch makes sure that we count freed paths when chase_all_symlinks() fails and we do not return directly. Also go ahead and remove assert() checks against mount counts being zero. This does not make sense since we are explicitly decrementing and freeing mount paths which makes it easy to hit the assert. If users did provide a misconfigured paths then just log a debug message, ignore and continue. Note that if mount count is zero at the beginning, then we we never hit this code path. Fixes systemd#4567
tixxdz
added a commit
to endocode/systemd
that referenced
this issue
Nov 9, 2016
This certainly fixes a bug that was introduced by PR systemd#4594 that intended to fix systemd#4567. The fix was not complete. This patch makes sure that we count and free all paths that fail inside chase_all_symlinks(). Fixes systemd#4567
keszybz
pushed a commit
to systemd/systemd-stable
that referenced
this issue
Jan 31, 2017
Instead of having two fields inside BindMount struct where one is stack based and the other one is heap, use one field to store the full path and updated it when we chase symlinks. This way we avoid dealing with both at the same time. This makes RootDirectory= work with ProtectHome= and ProtectKernelModules=yes Fixes: systemd/systemd#4567 (cherry picked from commit f0a4feb)
keszybz
pushed a commit
to systemd/systemd-stable
that referenced
this issue
Jan 31, 2017
…() (#4619) This certainly fixes a bug that was introduced by PR systemd/systemd#4594 that intended to fix systemd/systemd#4567. The fix was not complete. This patch makes sure that we count and free all paths that fail inside chase_all_symlinks(). Fixes systemd/systemd#4567 (cherry picked from commit 1d54cd5)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Based on #4533 (comment)
How to reproduce:
The text was updated successfully, but these errors were encountered: