[PATCH 1/1] copy_file(): Revise completion of SELinux security context preserve/set.
Xabier Oneca -- xOneca
xoneca at gmail.com
Wed Mar 25 22:24:46 UTC 2020
Hi Chris,
Thanks for the contribution.
Just one comment: please use bb_simple_perror_msg() instead of
bb_perror_msg() for single parameter uses (see commit:
https://git.busybox.net/busybox/commit/?id=0c97c9d43707)
Cheers,
> The existing setfscreatecon() at the beginning of copy_file() is the secure
> method for setting the context of new files, but it doesn't apply to
> existing files. Change the setfilecon() to only run on preexisting files.
>
> Signed-off-by: Chris PeBenito <chpebeni at linux.microsoft.com>
> ---
> libbb/copy_file.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/libbb/copy_file.c b/libbb/copy_file.c
> index 49d1ec9c6..37faa8dc6 100644
> --- a/libbb/copy_file.c
> +++ b/libbb/copy_file.c
> @@ -325,19 +325,22 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
> if ((flags & (FILEUTILS_PRESERVE_SECURITY_CONTEXT|FILEUTILS_SET_SECURITY_CONTEXT))
> && is_selinux_enabled() > 0
> ) {
> + /* Failure to preserve the security context isn't fatal here since
> + * the copy has been done at this point. */
> security_context_t con;
> - if (getfscreatecon(&con) == -1) {
> - bb_simple_perror_msg("getfscreatecon");
> - return -1;
> - }
> - if (con) {
> - if (setfilecon(dest, con) == -1) {
> + if (getfscreatecon(&con) < 0)
> + bb_perror_msg("getfscreatecon");
> +
> + if (setfscreatecon(NULL) < 0)
> + bb_perror_msg("can't reset fscreate");
> +
> + /* setfscreatecon() only works when a file is created. If dest
> + * preexisted, use setfilecon instead */
> + if (con && dest_exists)
> + if (setfilecon(dest, con) < 0)
> bb_perror_msg("setfilecon:%s,%s", dest, con);
> - freecon(con);
> - return -1;
> - }
> - freecon(con);
> - }
> +
> + freecon(con);
> }
> #endif
> #if ENABLE_FEATURE_CP_REFLINK
> --
> 2.21.1
More information about the busybox
mailing list