9:57 a.m.
The code at the very bottom of the DAC secdriver that calls
chown() should be fine with read-only data. If something needs to
be prepared it should have been done beforehand.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 28 ++++++++++++++++++++--------
src/security/security_dac.c | 4 ++--
src/security/security_manager.h | 2 +-
src/storage/storage_backend.h | 2 +-
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_backend_gluster.c | 2 +-
src/storage/storage_driver.c | 6 +++---
src/storage/storage_driver.h | 4 ++--
src/util/virstoragefile.c | 2 +-
src/util/virstoragefile.h | 2 +-
10 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1a464337e..f68b6ff30 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -329,10 +329,11 @@ qemuAutostartDomains(virQEMUDriverPtr driver)
static int
-qemuSecurityChownCallback(virStorageSourcePtr src,
+qemuSecurityChownCallback(const virStorageSource *storage,
uid_t uid,
gid_t gid)
{
+ virStorageSourcePtr src = NULL;
struct stat sb;
int save_errno = 0;
int ret = -1;
@@ -340,26 +341,35 @@ qemuSecurityChownCallback(virStorageSourcePtr src,
if (!virStorageFileSupportsSecurityDriver(src))
return 0;
+ if (!(src = virStorageSourceCopy(storage, false)))
+ goto cleanup;
+
if (virStorageSourceIsLocalStorage(src)) {
/* use direct chmod for local files so that the file doesn't
* need to be initialized */
- if (!src->path)
- return 0;
+ if (!src->path) {
+ ret = 0;
+ goto cleanup;
+ }
if (stat(src->path, &sb) >= 0) {
if (sb.st_uid == uid &&
sb.st_gid == gid) {
/* It's alright, there's nothing to change anyway. */
- return 0;
+ ret = 0;
+ goto cleanup;
}
}
- return chown(src->path, uid, gid);
+ ret = chown(src->path, uid, gid);
+ goto cleanup;
}
/* storage file init reports errors, return -2 on failure */
- if (virStorageFileInit(src) < 0)
- return -2;
+ if (virStorageFileInit(src) < 0) {
+ ret = -2;
+ goto cleanup;
+ }
if (virStorageFileChown(src, uid, gid) < 0) {
save_errno = errno;
@@ -370,7 +380,9 @@ qemuSecurityChownCallback(virStorageSourcePtr src,
cleanup:
virStorageFileDeinit(src);
- errno = save_errno;
+ virStorageSourceFree(src);
+ if (save_errno)
+ errno = save_errno;
return ret;
}
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 649219e52..b6f75fe1d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -279,8 +279,8 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
}
static int
-virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
- virStorageSourcePtr src,
+virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
+ const virStorageSource *src,
const char *path,
uid_t uid,
gid_t gid)
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 0d22cb15f..80fceddc8 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -62,7 +62,7 @@ int virSecurityManagerStackAddNested(virSecurityManagerPtr stack,
* @src. The callback shall return 0 on success, -1 on error and errno set (no
* libvirt error reported) OR -2 and a libvirt error reported. */
typedef int
-(*virSecurityManagerDACChownCallback)(virStorageSourcePtr src,
+(*virSecurityManagerDACChownCallback)(const virStorageSource *src,
uid_t uid,
gid_t gid);
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 28e1a6517..82fbbbf6d 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -285,7 +285,7 @@ typedef int
int mode);
typedef int
-(*virStorageFileBackendChown)(virStorageSourcePtr src,
+(*virStorageFileBackendChown)(const virStorageSource *src,
uid_t uid,
gid_t gid);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de0e8d57d..9de0c17c0 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1600,7 +1600,7 @@ virStorageFileBackendFileAccess(virStorageSourcePtr src,
static int
-virStorageFileBackendFileChown(virStorageSourcePtr src,
+virStorageFileBackendFileChown(const virStorageSource *src,
uid_t uid,
gid_t gid)
{
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 8e867043e..ae0611543 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -809,7 +809,7 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr
src)
static int
-virStorageFileBackendGlusterChown(virStorageSourcePtr src,
+virStorageFileBackendGlusterChown(const virStorageSource *src,
uid_t uid,
gid_t gid)
{
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index a79acc6f0..1ac07dbf2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2832,7 +2832,7 @@ int storageRegister(void)
/* ----------- file handlers cooperating with storage driver --------------- */
static bool
-virStorageFileIsInitialized(virStorageSourcePtr src)
+virStorageFileIsInitialized(const virStorageSource *src)
{
return src && src->drv;
}
@@ -2872,7 +2872,7 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr
src)
* driver to perform labelling
*/
bool
-virStorageFileSupportsSecurityDriver(virStorageSourcePtr src)
+virStorageFileSupportsSecurityDriver(const virStorageSource *src)
{
int actualType;
virStorageFileBackendPtr backend;
@@ -3163,7 +3163,7 @@ virStorageFileAccess(virStorageSourcePtr src,
* by libvirt storage backend.
*/
int
-virStorageFileChown(virStorageSourcePtr src,
+virStorageFileChown(const virStorageSource *src,
uid_t uid,
gid_t gid)
{
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index 3f2549da5..682c9ff82 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -44,9 +44,9 @@ ssize_t virStorageFileReadHeader(virStorageSourcePtr src,
char **buf);
const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src);
int virStorageFileAccess(virStorageSourcePtr src, int mode);
-int virStorageFileChown(virStorageSourcePtr src, uid_t uid, gid_t gid);
+int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid);
-bool virStorageFileSupportsSecurityDriver(virStorageSourcePtr src);
+bool virStorageFileSupportsSecurityDriver(const virStorageSource *src);
int virStorageFileGetMetadata(virStorageSourcePtr src,
uid_t uid, gid_t gid,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ce6d21388..3d4bda700 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2082,7 +2082,7 @@ virStorageSourceGetActualType(const virStorageSource *def)
bool
-virStorageSourceIsLocalStorage(virStorageSourcePtr src)
+virStorageSourceIsLocalStorage(const virStorageSource *src)
{
virStorageType type = virStorageSourceGetActualType(src);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 1f62244db..bea1c35a2 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -349,7 +349,7 @@ int virStorageSourceInitChainElement(virStorageSourcePtr newelem,
void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def);
void virStorageSourceClear(virStorageSourcePtr def);
int virStorageSourceGetActualType(const virStorageSource *def);
-bool virStorageSourceIsLocalStorage(virStorageSourcePtr src);
+bool virStorageSourceIsLocalStorage(const virStorageSource *src);
bool virStorageSourceIsEmpty(virStorageSourcePtr src);
bool virStorageSourceIsBlockLocal(const virStorageSource *src);
void virStorageSourceFree(virStorageSourcePtr def);
--
2.11.0