From patchwork Wed Mar 9 17:39:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 63723 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp2779799lbc; Wed, 9 Mar 2016 09:42:21 -0800 (PST) X-Received: by 10.28.172.194 with SMTP id v185mr26165680wme.21.1457545337646; Wed, 09 Mar 2016 09:42:17 -0800 (PST) Return-Path: Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com. [209.132.183.24]) by mx.google.com with ESMTPS id 5si28408242wmw.30.2016.03.09.09.42.16 (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 09 Mar 2016 09:42:17 -0800 (PST) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.24 as permitted sender) client-ip=209.132.183.24; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.24 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u29Hd8Yu025453; Wed, 9 Mar 2016 12:39:09 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u29Hd8Oj024041 for ; Wed, 9 Mar 2016 12:39:08 -0500 Received: from colepc.redhat.com (ovpn-113-126.phx2.redhat.com [10.3.113.126]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u29Hd28n003895; Wed, 9 Mar 2016 12:39:07 -0500 From: Cole Robinson To: libvirt-list@redhat.com Date: Wed, 9 Mar 2016 12:39:00 -0500 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-loop: libvir-list@redhat.com Cc: Pavel Hrdina Subject: [libvirt] [PATCH 3/3] util: virfile: Only setuid for virFileRemove if on NFS X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com NFS with root-squash is the only reason we need to do setuid/setgid crazyness in virFileRemove, so limit that behavior to the NFS case. --- I'm not sure though if NFS is the only case we care about this here, or if we want to conditionalize this path on NFS since that makes it more of a pain to test... It's not required to fix the initial bug src/util/virfile.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) -- 2.5.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list diff --git a/src/util/virfile.c b/src/util/virfile.c index cea2674..3d1b118 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2322,7 +2322,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, * owned by the passed uid/gid pair. Needed for NFS with root-squash */ static bool -virFileRemoveNeedsSetuid(uid_t uid, gid_t gid) +virFileRemoveNeedsSetuid(const char *path, uid_t uid, gid_t gid) { /* If running unprivileged, setuid isn't going to work */ if (geteuid() != 0) @@ -2336,6 +2336,12 @@ virFileRemoveNeedsSetuid(uid_t uid, gid_t gid) if (uid == geteuid() && gid == getegid()) return false; + /* Only perform the setuid stuff for NFS, which is the only case + that may actually need it. This can error, but just be safe and + only check for a clear negative result. */ + if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 0) + return false; + return true; } @@ -2361,7 +2367,7 @@ virFileRemove(const char *path, gid_t *groups; int ngroups; - if (!virFileRemoveNeedsSetuid(uid, gid)) { + if (!virFileRemoveNeedsSetuid(path, uid, gid)) { if (virFileIsDir(path)) return rmdir(path); else