[libvirt] [PATCH 00/10] Resolve some Coverity found issues

After installing a new version of Coverity - my daily checker has found many more (>140) "new" issues. Figured I'd start tackling the list bit by bit. I didn't dig into when any of these were introduced - with so many on the list it'd be overwhelming :-) John Ferlan (10): parallels: Resolve Coverity USE_AFTER_FREE xen_common: Resolve Coverity USE_AFTER_FREE xen_xm: Resolve Coverity USE_AFTER_FREE storage_driver: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_command: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_driver: Resolve Coverity CONSTANT_EXPRESSION_RESULT libxl_migration: Resolve Coverity NULL_RETURNS qemu_capabilities: Resolve Coverity NULL_RETURNS src/conf/domain_conf.c | 18 +++++++++--------- src/libxl/libxl_migration.c | 1 - src/parallels/parallels_network.c | 4 +--- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_driver.c | 6 ++++-- src/xenconfig/xen_common.c | 5 ++--- src/xenconfig/xen_xm.c | 1 + 9 files changed, 24 insertions(+), 22 deletions(-) -- 1.9.3

Coverity complains that calling virNetworkDefFree(def), then jumping to the cleanup: label which calls virNetworkDefFree(def) could result in a double_free. Just remove the call from the if statement. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/parallels/parallels_network.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index a45acdc..f41c97f 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -226,10 +226,8 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; } - if (!(net = virNetworkAssignDef(&privconn->networks, def, false))) { - virNetworkDefFree(def); + if (!(net = virNetworkAssignDef(&privconn->networks, def, false))) goto cleanup; - } net->active = 1; net->autostart = 1; virNetworkObjUnlock(net); -- 1.9.3

There were two warnings in this module If the VIR_ALLOC_N(def->serials, 1) fails, then a virDomainChrDefFree(chr) is called and we jump to cleanup which makes the same call. Just remove the one after VIR_ALLOC_N() In the label "skipnic:" a virDomainNetDefFree(net) is made; however, if in going back to the top of the loop we jump back down to skipnic for any reason, the call will attempt to free an already freed structure since "net" was not passed by reference to virDomainNetDefFree(). Just set net = NULL in skipnic: to resolve the issue. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/xenconfig/xen_common.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 2f53c63..9beaf6c 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -776,10 +776,8 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def) !(chr = xenParseSxprChar(str, NULL))) goto cleanup; if (chr) { - if (VIR_ALLOC_N(def->serials, 1) < 0) { - virDomainChrDefFree(chr); + if (VIR_ALLOC_N(def->serials, 1) < 0) goto cleanup; - } chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; chr->target.port = 0; def->serials[0] = chr; @@ -953,6 +951,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) skipnic: list = list->next; virDomainNetDefFree(net); + net = NULL; VIR_FREE(script); } } -- 1.9.3

If virDomainDiskDefFree(disk) is called in 'skipdisk:', then it's possible to either return to skipdisk without reallocating a new disk (via the if condition just prior) or to end the loop having deleted the disk. Since virDomainDiskDefFree() does not pass by reference, disk isn't changed in this context, thus the possible issue. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/xenconfig/xen_xm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index fe46d91..1e57e24 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -201,6 +201,7 @@ xenParseXMDisk(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) skipdisk: list = list->next; virDomainDiskDefFree(disk); + disk = NULL; } } -- 1.9.3

There were two occurrances of attempting to initialize actualType by calling virStorageSourceGetActualType(src) prior to a check if (!src) resulting in Coverity complaining about the possible NULL dereference in virStorageSourceGetActualType() of src. Resolve by moving the actualType setting until after checking !src Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 5ddc23a..433d7b7 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2441,11 +2441,12 @@ virStorageFileIsInitialized(virStorageSourcePtr src) static bool virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) { - int actualType = virStorageSourceGetActualType(src); + int actualType; virStorageFileBackendPtr backend; if (!src) return false; + actualType = virStorageSourceGetActualType(src); if (src->drv) { backend = src->drv->backend; @@ -2473,11 +2474,12 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) bool virStorageFileSupportsSecurityDriver(virStorageSourcePtr src) { - int actualType = virStorageSourceGetActualType(src); + int actualType; virStorageFileBackendPtr backend; if (!src) return false; + actualType = virStorageSourceGetActualType(src); if (src->drv) { backend = src->drv->backend; -- 1.9.3

On 08/27/14 15:51, John Ferlan wrote:
There were two occurrances of attempting to initialize actualType by calling virStorageSourceGetActualType(src) prior to a check if (!src) resulting in Coverity complaining about the possible NULL dereference in virStorageSourceGetActualType() of src.
Resolve by moving the actualType setting until after checking !src
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ACK, safe for 1.2.8 Peter

In virDomainActualNetDefFormat() a call to virDomainNetGetActualType(def) was made before a check for (!def) a few lines later. This triggered Coverity to note the possible NULL deref. Just moving the initialization to after the !def checks resolves the issue Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3b295ab..f096bcf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16207,11 +16207,13 @@ virDomainActualNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { - unsigned int type = virDomainNetGetActualType(def); - const char *typeStr = virDomainNetTypeToString(type); + unsigned int type; + const char *typeStr; if (!def) return 0; + type = virDomainNetGetActualType(def); + typeStr = virDomainNetTypeToString(type); if (!typeStr) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 1.9.3

In qemuNetworkIfaceConnect() a call to virNetDevBandwidthSet() is made where the function prototype requires the first parameter (net->ifname) to be non NULL. Coverity complains that the subsequent non NULL check for net->ifname prior to the next call gets flagged as an unnecessary check. Resolve by removing the extra check Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9241f57..8fb81a4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -370,7 +370,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, false) < 0) goto cleanup; - if (net->filter && net->ifname && + if (net->filter && virDomainConfNWFilterInstantiate(conn, def->uuid, net) < 0) { goto cleanup; } -- 1.9.3

Coverity complains that checking for domain->def being non NULL in the if (live) path of virDomainObjAssignDef() would be unnecessary or a NULL deref since the call to virDomainObjIsActive() would already dereference domain->def when checking if the def->id field was != -1. Checked all callers to virDomainObjAssignDef() and each at some point dereferences (vm)->def->{field} prior to calling when live is true. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f096bcf..afe6a06 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2243,13 +2243,11 @@ void virDomainObjAssignDef(virDomainObjPtr domain, domain->newDef = def; } else { if (live) { - if (domain->def) { - /* save current configuration to be restored on domain shutdown */ - if (!domain->newDef) - domain->newDef = domain->def; - else - virDomainDefFree(domain->def); - } + /* save current configuration to be restored on domain shutdown */ + if (!domain->newDef) + domain->newDef = domain->def; + else + virDomainDefFree(domain->def); domain->def = def; } else { if (oldDef) -- 1.9.3

The call to virDomainSnapshotRedefinePrep() had a spurrious ! in front of it which caused Coverity to complan that the expression is always false. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2db507f..73959da 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13461,8 +13461,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } if (redefine) { - if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, - &update_current, flags) < 0) + if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + &update_current, flags) < 0) goto cleanup; } else { /* Easiest way to clone inactive portion of vm->def is via -- 1.9.3

Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that two events would have been sent which more than likely causes some bad stuff for the second one. So just remove one call and let the cleanup: handle the event. In the future if there's code between getting the event and cleanup that needs to send the event, this will have to change in order to send the event and set event = NULL (although it seems unlikely to happen). Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..eb65536 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -519,7 +519,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - libxlDomainEventQueue(driver, event); } cleanup: -- 1.9.3

On 08/27/2014 03:51 PM, John Ferlan wrote:
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that two events would have been sent which more than likely causes some bad stuff for the second one. So just remove one call and let the cleanup: handle the event. In the future if there's code between getting the event and cleanup that needs to send the event, this will have to change in order to send the event and set event = NULL (although it seems unlikely to happen).
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..eb65536 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -519,7 +519,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED);
This (possibly) overwrites the event created earlier. But I'm not sure if the right solution is to send both events, or clear the original one. Jan
- libxlDomainEventQueue(driver, event); }
cleanup:

On 08/27/2014 10:42 AM, Ján Tomko wrote:
On 08/27/2014 03:51 PM, John Ferlan wrote:
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that two events would have been sent which more than likely causes some bad stuff for the second one. So just remove one call and let the cleanup: handle the event. In the future if there's code between getting the event and cleanup that needs to send the event, this will have to change in order to send the event and set event = NULL (although it seems unlikely to happen).
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..eb65536 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -519,7 +519,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED);
This (possibly) overwrites the event created earlier. But I'm not sure if the right solution is to send both events, or clear the original one.
Jan
Hmmm.. right Maybe a: if (event) { libxlDomainEventQueue(driver, event); event = NULL; } before the: dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); will at least cause the RESUME/SUSPEND events to be sent and then if the dom == NULL the STOPPED event would be sent as well I've copied Jim Fehlig for his opinion - since he wrote the code... John
- libxlDomainEventQueue(driver, event); }
cleanup:

John Ferlan wrote:
On 08/27/2014 10:42 AM, Ján Tomko wrote:
On 08/27/2014 03:51 PM, John Ferlan wrote:
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that two events would have been sent which more than likely causes some bad stuff for the second one. So just remove one call and let the cleanup: handle the event. In the future if there's code between getting the event and cleanup that needs to send the event, this will have to change in order to send the event and set event = NULL (although it seems unlikely to happen).
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..eb65536 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -519,7 +519,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED);
This (possibly) overwrites the event created earlier. But I'm not sure if the right solution is to send both events, or clear the original one.
Jan
Hmmm.. right
Maybe a:
if (event) { libxlDomainEventQueue(driver, event); event = NULL; }
before the:
dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
Hrm, I wonder if it is even possible for dom to be NULL in this case? We are in the finish phase and have even unpaused the domain, if requested. I see that dom == NULL is never checked in qemuMigrationFinish(). Perhaps the whole 'if (dom == NULL)' check can be removed here too. Regards, Jim
will at least cause the RESUME/SUSPEND events to be sent and then if the dom == NULL the STOPPED event would be sent as well
I've copied Jim Fehlig for his opinion - since he wrote the code...
John
- libxlDomainEventQueue(driver, event); }
cleanup:

<...snip...>
Hmmm.. right
Maybe a:
if (event) { libxlDomainEventQueue(driver, event); event = NULL; }
before the:
dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
Hrm, I wonder if it is even possible for dom to be NULL in this case? We are in the finish phase and have even unpaused the domain, if requested. I see that dom == NULL is never checked in qemuMigrationFinish(). Perhaps the whole 'if (dom == NULL)' check can be removed here too.
Regards, Jim
OK - prior to reading this I just included this as patch 1 of the other Coverity pile I just dumped on the list. If you have an epiphany after reading the code again - let me know - I can adjust it... John

Adjust the initialization of qemuCaps() to check for a NULL before attempting to dereference like other callers/users do. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 410086b..ce899f2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3349,10 +3349,13 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, uid_t runUid, gid_t runGid) { - virQEMUCapsPtr qemuCaps = virQEMUCapsNew(); + virQEMUCapsPtr qemuCaps; struct stat sb; int rv; + if (!(qemuCaps = virQEMUCapsNew())) + goto error; + if (VIR_STRDUP(qemuCaps->binary, binary) < 0) goto error; -- 1.9.3

On 08/27/2014 03:51 PM, John Ferlan wrote:
After installing a new version of Coverity - my daily checker has found many more (>140) "new" issues. Figured I'd start tackling the list bit by bit. I didn't dig into when any of these were introduced - with so many on the list it'd be overwhelming :-)
John Ferlan (10): parallels: Resolve Coverity USE_AFTER_FREE xen_common: Resolve Coverity USE_AFTER_FREE xen_xm: Resolve Coverity USE_AFTER_FREE storage_driver: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_command: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_driver: Resolve Coverity CONSTANT_EXPRESSION_RESULT qemu_capabilities: Resolve Coverity NULL_RETURNS
ACK to patches 1-8, 10. Jan
libxl_migration: Resolve Coverity NULL_RETURNS
src/conf/domain_conf.c | 18 +++++++++--------- src/libxl/libxl_migration.c | 1 - src/parallels/parallels_network.c | 4 +--- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_driver.c | 6 ++++-- src/xenconfig/xen_common.c | 5 ++--- src/xenconfig/xen_xm.c | 1 + 9 files changed, 24 insertions(+), 22 deletions(-)

On 08/27/2014 08:42 AM, Ján Tomko wrote:
On 08/27/2014 03:51 PM, John Ferlan wrote:
After installing a new version of Coverity - my daily checker has found many more (>140) "new" issues. Figured I'd start tackling the list bit by bit. I didn't dig into when any of these were introduced - with so many on the list it'd be overwhelming :-)
John Ferlan (10): parallels: Resolve Coverity USE_AFTER_FREE xen_common: Resolve Coverity USE_AFTER_FREE xen_xm: Resolve Coverity USE_AFTER_FREE storage_driver: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_command: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_driver: Resolve Coverity CONSTANT_EXPRESSION_RESULT qemu_capabilities: Resolve Coverity NULL_RETURNS
ACK to patches 1-8, 10.
And add my ACK to patch 9; series is good to go. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/27/2014 08:59 AM, Eric Blake wrote:
On 08/27/2014 08:42 AM, Ján Tomko wrote:
On 08/27/2014 03:51 PM, John Ferlan wrote:
After installing a new version of Coverity - my daily checker has found many more (>140) "new" issues. Figured I'd start tackling the list bit by bit. I didn't dig into when any of these were introduced - with so many on the list it'd be overwhelming :-)
John Ferlan (10): parallels: Resolve Coverity USE_AFTER_FREE xen_common: Resolve Coverity USE_AFTER_FREE xen_xm: Resolve Coverity USE_AFTER_FREE storage_driver: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_command: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_driver: Resolve Coverity CONSTANT_EXPRESSION_RESULT qemu_capabilities: Resolve Coverity NULL_RETURNS
ACK to patches 1-8, 10.
And add my ACK to patch 9; series is good to go.
Scratch that, I missed on patch 9 that event was already in use (curse my inbox for delivering mail out-of-order today). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/27/2014 09:51 AM, John Ferlan wrote:
After installing a new version of Coverity - my daily checker has found many more (>140) "new" issues. Figured I'd start tackling the list bit by bit. I didn't dig into when any of these were introduced - with so many on the list it'd be overwhelming :-)
John Ferlan (10): parallels: Resolve Coverity USE_AFTER_FREE xen_common: Resolve Coverity USE_AFTER_FREE xen_xm: Resolve Coverity USE_AFTER_FREE storage_driver: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_command: Resolve Coverity REVERSE_INULL domain_conf: Resolve Coverity REVERSE_INULL qemu_driver: Resolve Coverity CONSTANT_EXPRESSION_RESULT libxl_migration: Resolve Coverity NULL_RETURNS qemu_capabilities: Resolve Coverity NULL_RETURNS
src/conf/domain_conf.c | 18 +++++++++--------- src/libxl/libxl_migration.c | 1 - src/parallels/parallels_network.c | 4 +--- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_driver.c | 6 ++++-- src/xenconfig/xen_common.c | 5 ++--- src/xenconfig/xen_xm.c | 1 + 9 files changed, 24 insertions(+), 22 deletions(-)
I pushed 1-8 & 10 - will await more feedback on 9. Besides there are another 140 go to! Thanks - John
participants (5)
-
Eric Blake
-
Jim Fehlig
-
John Ferlan
-
Ján Tomko
-
Peter Krempa