diff --git a/ChangeLog b/ChangeLog index 1afec5929e..9f939b64f4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +Wed Apr 1 11:22:22 BST 2009 Daniel P. Berrange + + Sanitise symlink resolving + * src/libvirt_private.syms: Add virFileResolveLink + * src/util.c, src/util.h: Add convenient virFileResolveLink + for reading symlink destination safely + * src/storage_backend_disk.c, src/security_selinux.c: Switch + over to calling virFileResolveLink + Wed Apr 1 11:18:22 BST 2009 Daniel P. Berrange Misc memory handling fixes diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a5f9f925a6..78f093c196 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -306,6 +306,7 @@ virStrToLong_ll; virStrToLong_ull; virStrToLong_ui; virFileLinkPointsTo; +virFileResolveLink; saferead; safewrite; safezero; diff --git a/src/security_selinux.c b/src/security_selinux.c index 1708d55082..091fe6ee5a 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -293,28 +293,24 @@ SELinuxRestoreSecurityImageLabel(virConnectPtr conn, struct stat buf; security_context_t fcon = NULL; int rc = -1; + int err; char *newpath = NULL; const char *path = disk->src; if (disk->readonly || disk->shared) return 0; - if (lstat(path, &buf) != 0) - return -1; - - if (S_ISLNK(buf.st_mode)) { - if (VIR_ALLOC_N(newpath, buf.st_size + 1) < 0) - return -1; - - if (readlink(path, newpath, buf.st_size) < 0) - goto err; - path = newpath; - if (stat(path, &buf) != 0) - goto err; + if ((err = virFileResolveLink(path, &newpath)) < 0) { + virReportSystemError(conn, err, + _("cannot resolve symlink %s"), path); + goto err; } - if (matchpathcon(path, buf.st_mode, &fcon) == 0) { - rc = SELinuxSetFilecon(conn, path, fcon); + if (stat(newpath, &buf) != 0) + goto err; + + if (matchpathcon(newpath, buf.st_mode, &fcon) == 0) { + rc = SELinuxSetFilecon(conn, newpath, fcon); } err: VIR_FREE(fcon); diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c index 9280906709..4b0da3d7d4 100644 --- a/src/storage_backend_disk.c +++ b/src/storage_backend_disk.c @@ -362,20 +362,16 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, unsigned int flags ATTRIBUTE_UNUSED) { char *part_num = NULL; - int n; - char devpath[PATH_MAX]; + int err; + char *devpath = NULL; char *devname, *srcname; + int rc = -1; - if ((n = readlink(vol->target.path, devpath, sizeof(devpath))) < 0 && - errno != EINVAL) { - virReportSystemError(conn, errno, + if ((err = virFileResolveLink(vol->target.path, &devpath)) < 0) { + virReportSystemError(conn, err, _("Couldn't read volume target path '%s'"), vol->target.path); - return -1; - } else if (n <= 0) { - strncpy(devpath, vol->target.path, PATH_MAX); - } else { - devpath[n] = '\0'; + goto cleanup; } devname = basename(devpath); @@ -386,7 +382,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("Volume path '%s' did not start with parent " "pool source device name."), devname); - return -1; + goto cleanup; } part_num = devname + strlen(srcname); @@ -395,7 +391,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("cannot parse partition number from target " "'%s'"), devname); - return -1; + goto cleanup; } /* eg parted /dev/sda rm 2 */ @@ -409,9 +405,12 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, }; if (virRun(conn, prog, NULL) < 0) - return -1; + goto cleanup; - return 0; + rc = 0; +cleanup: + VIR_FREE(devpath); + return rc; } diff --git a/src/util.c b/src/util.c index 016cb626d7..65088b04c5 100644 --- a/src/util.c +++ b/src/util.c @@ -937,6 +937,53 @@ int virFileLinkPointsTo(const char *checkLink, && SAME_INODE (src_sb, dest_sb)); } + + +/* + * Attempt to resolve a symbolic link, returning the + * real path + * + * Return 0 if path was not a symbolic, or the link was + * resolved. Return -1 upon error + */ +int virFileResolveLink(const char *linkpath, + char **resultpath) +{ + struct stat st; + char *buf; + int n; + + *resultpath = NULL; + + if (lstat(linkpath, &st) < 0) + return errno; + + if (!S_ISLNK(st.st_mode)) { + if (!(*resultpath = strdup(linkpath))) + return -ENOMEM; + return 0; + } + + /* Posix says that 'st_size' field from + * result of an lstat() call is filled with + * number of bytes in the destination + * filename. + */ + if (VIR_ALLOC_N(buf, st.st_size + 1) < 0) + return -ENOMEM; + + if ((n = readlink(linkpath, buf, st.st_size)) < 0) { + VIR_FREE(buf); + return -errno; + } + + buf[n] = '\0'; + + *resultpath = buf; + return 0; +} + + int virFileExists(const char *path) { struct stat st; diff --git a/src/util.h b/src/util.h index 3fd5d2511c..6fe03b634f 100644 --- a/src/util.h +++ b/src/util.h @@ -87,6 +87,9 @@ int virFileStripSuffix(char *str, int virFileLinkPointsTo(const char *checkLink, const char *checkDest); +int virFileResolveLink(const char *linkpath, + char **resultpath); + int virFileExists(const char *path); int virFileMakePath(const char *path);