On Wed, Dec 06, 2017 at 16:39:27 -0500, John Ferlan wrote:

On 12/06/2017 10:28 AM, Peter Krempa wrote:

...

virStorageFileReportBrokenChain uses data from the driver private data pointer to print the user and group. This would lead to a crash in call paths where we did not initialize the storage backend as recently added in commit 24e47ee2b93 to qemuDomainDetermineDiskChain.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1522682 ---

Once pushed I'll also backport it to a maint branch of the last release if that's desired.

src/storage/storage_source.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index 4586ef4ad4..a5df84bae3 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -419,19 +419,33 @@ virStorageFileReportBrokenChain(int errcode, virStorageSourcePtr src, virStorageSourcePtr parent) { - unsigned int access_user = src->drv->uid; - unsigned int access_group = src->drv->gid; - - if (src == parent) { - virReportSystemError(errcode, - _("Cannot access storage file '%s' " - "(as uid:%u, gid:%u)"), - src->path, access_user, access_group); + + if (src->drv) { + unsigned int access_user = src->drv->uid; + unsigned int access_group = src->drv->gid; + + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s' " + "(as uid:%u, gid:%u)"), + src->path, access_user, access_group); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s' (as uid:%u, gid:%u)"), + src->path, parent->path, access_user, access_group); + } } else { - virReportSystemError(errcode, - _("Cannot access backing file '%s' " - "of storage file '%s' (as uid:%u, gid:%u)"), - src->path, parent->path, access_user, access_group); + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s' "), ^ Not that it matters much because it probably won't be seen, but you could drop the trailing space.

Right, I've just deleted the second line without fixing the whitespace.

Reviewed-by: John Ferlan <jferlan@redhat.com>

Thanks.

John

...

+ src->path); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s'"), + src->path, parent->path); + } } }