From patchwork Thu Mar 24 09:52:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Petr Lautrbach X-Patchwork-Id: 12790584 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF989C433F5 for ; Thu, 24 Mar 2022 09:53:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244270AbiCXJyo (ORCPT ); Thu, 24 Mar 2022 05:54:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52580 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349089AbiCXJyi (ORCPT ); Thu, 24 Mar 2022 05:54:38 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 5030C6544 for ; Thu, 24 Mar 2022 02:53:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648115584; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Vbb20GdGEX3bKiyJXVgyCCMKnidyxPKPQOYdD3bc9qU=; b=ULUFUcaOsFp2wn6LMFaV8Gp2gvw8agNWjLrXcNnIdyj++Sf1H3EdOEO/rPtuxbSyW3lJtW EaXYwYXha0fcK8F52SK5IGdZ52qDkLVOu6EuEZkrqyr+YY5U2+pEjd41jzNtbyENy4n3bX Cw20afrURixZUVmE9bN8W7nWndrwi4Y= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-367-b5KqB_n9OGmudMjLXUlMZA-1; Thu, 24 Mar 2022 05:53:03 -0400 X-MC-Unique: b5KqB_n9OGmudMjLXUlMZA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3FC1D3C16187 for ; Thu, 24 Mar 2022 09:53:03 +0000 (UTC) Received: from localhost.localdomain (unknown [10.40.192.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6F50941D3F9; Thu, 24 Mar 2022 09:53:02 +0000 (UTC) From: Petr Lautrbach To: selinux@vger.kernel.org Cc: Petr Lautrbach , Joseph Marrero Corchado Subject: [PATCH v2] libsemanage: Fall back to semanage_copy_dir when rename() fails Date: Thu, 24 Mar 2022 10:52:51 +0100 Message-Id: <20220324095251.1561597-1-plautrba@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.9 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org In some circumstances, like semanage-store being on overlayfs, rename() could fail with EXDEV - Invalid cross-device link. This is due to the fact that overlays doesn't support rename() if source and target are not on the same layer, e.g. in containers built from several layers. Even though it's not atomic operation, it's better to try to copy files from src to dst on our own in this case. Next rebuild will probably not fail as the new directories will be on the same layer. Fixes: https://github.com/SELinuxProject/selinux/issues/343 Reproducer: $ cd selinux1 $ cat Dockerfile FROM fedora:35 RUN dnf install -y selinux-policy selinux-policy-targeted $ podman build -t localhost/selinux . --no-cache $ cd ../selinux2 $ cat Dockerfile FROM localhost/selinux RUN semodule -B $ podman build -t localhost/selinux2 . --no-cache STEP 2/2: RUN semodule -B libsemanage.semanage_commit_sandbox: Error while renaming /var/lib/selinux/targeted/active to /var/lib/selinux/targeted/previous. (Invalid cross-device link). semodule: Failed! Error: error building at STEP "RUN semodule -B": error while running runtime: exit status 1 With the fix: $ podman build -t localhost/selinux2 . --no-cache STEP 2/2: RUN semodule -B libsemanage.semanage_rename: Warning: rename(/var/lib/selinux/targeted/active, /var/lib/selinux/targeted/previous) failed: Invalid cross-device link, fall back to non-atomic semanage_copy_dir_flags() COMMIT localhost/selinux2 --> d2cfcebc1a1 Successfully tagged localhost/selinux2:latest d2cfcebc1a1b34f1c2cd661ac18292b0612c3e5fa71d6fa1441be244da91b1af Reported-by: Joseph Marrero Corchado Signed-off-by: Petr Lautrbach --- changes to v1 base on Ondrej's feedback: - improve the commit message - use WARN() instead of fprintf(stderr, libsemanage/src/semanage_store.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c index 767f05cb2853..3ae60493c8e5 100644 --- a/libsemanage/src/semanage_store.c +++ b/libsemanage/src/semanage_store.c @@ -697,6 +697,10 @@ int semanage_store_access_check(void) /********************* other I/O functions *********************/ +static int semanage_rename(semanage_handle_t * sh, const char *tmp, const char *dst); +int semanage_remove_directory(const char *path); +static int semanage_copy_dir_flags(const char *src, const char *dst, int flag); + /* Callback used by scandir() to select files. */ static int semanage_filename_select(const struct dirent *d) { @@ -768,7 +772,20 @@ out: return retval; } -static int semanage_copy_dir_flags(const char *src, const char *dst, int flag); +static int semanage_rename(semanage_handle_t * sh, const char *src, const char *dst) { + int retval; + + retval = rename(src, dst); + if (retval == 0 || errno != EXDEV) + return retval; + + /* we can't use rename() due to filesystem limitation, lets try to copy files manually */ + WARN(sh, "Warning: rename(%s, %s) failed: %s, fall back to non-atomic semanage_copy_dir_flags()\n", src, dst, strerror(errno)); + if (semanage_copy_dir_flags(src, dst, 1) == -1) { + return -1; + } + return semanage_remove_directory(src); +} /* Copies all of the files from src to dst, recursing into * subdirectories. Returns 0 on success, -1 on error. */ @@ -1770,7 +1787,7 @@ static int semanage_commit_sandbox(semanage_handle_t * sh) goto cleanup; } - if (rename(active, backup) == -1) { + if (semanage_rename(sh, active, backup) == -1) { ERR(sh, "Error while renaming %s to %s.", active, backup); retval = -1; goto cleanup; @@ -1779,12 +1796,12 @@ static int semanage_commit_sandbox(semanage_handle_t * sh) /* clean up some files from the sandbox before install */ /* remove homedir_template from sandbox */ - if (rename(sandbox, active) == -1) { + if (semanage_rename(sh, sandbox, active) == -1) { ERR(sh, "Error while renaming %s to %s.", sandbox, active); /* note that if an error occurs during the next * function then the store will be left in an * inconsistent state */ - if (rename(backup, active) < 0) + if (semanage_rename(sh, backup, active) < 0) ERR(sh, "Error while renaming %s back to %s.", backup, active); retval = -1; @@ -1795,10 +1812,10 @@ static int semanage_commit_sandbox(semanage_handle_t * sh) * function then the store will be left in an * inconsistent state */ int errsv = errno; - if (rename(active, sandbox) < 0) + if (semanage_rename(sh, active, sandbox) < 0) ERR(sh, "Error while renaming %s back to %s.", active, sandbox); - else if (rename(backup, active) < 0) + else if (semanage_rename(sh, backup, active) < 0) ERR(sh, "Error while renaming %s back to %s.", backup, active); else From patchwork Fri Apr 1 13:37:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Petr Lautrbach X-Patchwork-Id: 12798308 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 005F6C433F5 for ; Fri, 1 Apr 2022 13:38:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234797AbiDANjw (ORCPT ); Fri, 1 Apr 2022 09:39:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346418AbiDANjt (ORCPT ); Fri, 1 Apr 2022 09:39:49 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 0A90927F4C3 for ; Fri, 1 Apr 2022 06:37:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648820278; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=S0HR60LhhaFKC4d8cwNO85OUXdRqEy6h6RfJ0akXPtc=; b=B/163IsIF0xPV4fAW3Fr9bFKYhTTwS5RLGPopmSbOP4EbGexpTW53NuY/gDLWzST4LKDzT 5Wf+O8JcAKMI8C+lOl4lKImWtyJBQYw0FrQbOot3gJlEznl7oncYACKYJCS7jMDXQJEE9v g79j+Ac1rb+UjGJh4kWGHU8zXfd96rw= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-144-vV6OAdUPNVa4Hm4zYP8f5Q-1; Fri, 01 Apr 2022 09:37:57 -0400 X-MC-Unique: vV6OAdUPNVa4Hm4zYP8f5Q-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8BC548002BF for ; Fri, 1 Apr 2022 13:37:57 +0000 (UTC) Received: from localhost.localdomain (unknown [10.40.194.37]) by smtp.corp.redhat.com (Postfix) with ESMTP id DD6CE53C9; Fri, 1 Apr 2022 13:37:56 +0000 (UTC) From: Petr Lautrbach To: selinux@vger.kernel.org Cc: Petr Lautrbach Subject: [PATCH v2 3/3] mcstrans: Fir RESOURCE_LEAK and USE_AFTER_FREE coverity scan defects Date: Fri, 1 Apr 2022 15:37:46 +0200 Message-Id: <20220401133746.122629-1-plautrba@redhat.com> In-Reply-To: <20220324095251.1561597-1-plautrba@redhat.com> References: <20220324095251.1561597-1-plautrba@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Signed-off-by: Petr Lautrbach --- mcstrans/src/mcstrans.c | 25 ++++++++++++++++++++++++- mcstrans/src/mcstransd.c | 4 +++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/mcstrans/src/mcstrans.c b/mcstrans/src/mcstrans.c index d42760fdbfc2..af3f507ef718 100644 --- a/mcstrans/src/mcstrans.c +++ b/mcstrans/src/mcstrans.c @@ -632,16 +632,23 @@ add_cache(domain_t *domain, char *raw, char *trans) { map->raw = strdup(raw); if (!map->raw) { + free(map); goto err; } map->trans = strdup(trans); if (!map->trans) { + free(map->raw); + free(map); goto err; } log_debug(" add_cache (%s,%s)\n", raw, trans); - if (add_to_hashtable(domain->raw_to_trans, map->raw, map) < 0) + if (add_to_hashtable(domain->raw_to_trans, map->raw, map) < 0) { + free(map->trans); + free(map->raw); + free(map); goto err; + } if (add_to_hashtable(domain->trans_to_raw, map->trans, map) < 0) goto err; @@ -1568,6 +1575,7 @@ trans_context(const char *incon, char **rcon) { trans = compute_trans_from_raw(range, domain); if (trans) if (add_cache(domain, range, trans) < 0) { + free(trans); free(range); return -1; } @@ -1579,6 +1587,7 @@ trans_context(const char *incon, char **rcon) { ltrans = compute_trans_from_raw(lrange, domain); if (ltrans) { if (add_cache(domain, lrange, ltrans) < 0) { + free(ltrans); free(range); return -1; } @@ -1597,6 +1606,7 @@ trans_context(const char *incon, char **rcon) { utrans = compute_trans_from_raw(urange, domain); if (utrans) { if (add_cache(domain, urange, utrans) < 0) { + free(utrans); free(ltrans); free(range); return -1; @@ -1636,6 +1646,10 @@ trans_context(const char *incon, char **rcon) { } if (dashp) *dashp = '-'; + if (trans) { + free(trans); + trans = NULL; + } } if (trans) { @@ -1696,7 +1710,9 @@ untrans_context(const char *incon, char **rcon) { canonical = compute_trans_from_raw(raw, domain); if (canonical && strcmp(canonical, range)) if (add_cache(domain, raw, canonical) < 0) { + free(canonical); free(range); + free(raw); return -1; } } @@ -1704,6 +1720,7 @@ untrans_context(const char *incon, char **rcon) { free(canonical); if (add_cache(domain, raw, range) < 0) { free(range); + free(raw); return -1; } } else { @@ -1721,6 +1738,7 @@ untrans_context(const char *incon, char **rcon) { canonical = compute_trans_from_raw(lraw, domain); if (canonical) if (add_cache(domain, lraw, canonical) < 0) { + free(canonical); free(lraw); free(range); return -1; @@ -1752,6 +1770,7 @@ untrans_context(const char *incon, char **rcon) { canonical = compute_trans_from_raw(uraw, domain); if (canonical) if (add_cache(domain, uraw, canonical) < 0) { + free(canonical); free(uraw); free(lraw); free(range); @@ -1802,6 +1821,10 @@ untrans_context(const char *incon, char **rcon) { } if (dashp) *dashp = '-'; + if (raw) { + free(raw); + raw = NULL; + } } if (raw) { diff --git a/mcstrans/src/mcstransd.c b/mcstrans/src/mcstransd.c index 536c0f32f23a..42262e580386 100644 --- a/mcstrans/src/mcstransd.c +++ b/mcstrans/src/mcstransd.c @@ -328,6 +328,7 @@ process_events(struct pollfd **ufds, int *nfds) /* Setup pollfd for deletion later. */ (*ufds)[ii].fd = -1; close(connfd); + connfd = -1; /* So we don't get bothered later */ revents = revents & ~(POLLHUP); } @@ -341,10 +342,11 @@ process_events(struct pollfd **ufds, int *nfds) /* Set the pollfd up for deletion later. */ (*ufds)[ii].fd = -1; close(connfd); + connfd = -1; revents = revents & ~(POLLHUP); } - if (revents) { + if (revents && connfd != -1) { syslog(LOG_ERR, "Unknown/error events (%x) encountered" " for fd (%d)\n", revents, connfd);