From 2b37e9f84372b5c98ef0ba114ce016953e47bc3c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Nov 2020 07:23:57 +0100 Subject: [PATCH 1/4] authz-list-file: Fix file read error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. qauthz_list_file_complete() is wrong that way: it passes @errp to qauthz_list_file_complete() without checking for failure. If it runs into another failure, it trips error_setv()'s assertion. Reproducer: $ qemu-system-x86_64 -nodefaults -S -display none -object authz-list-file,id=authz0,filename= qemu-system-x86_64: ../util/error.c:59: error_setv: Assertion `*errp == NULL' failed. Aborted (core dumped) Fix it to check for failure. Fixes: 55d869846de802a16af1a50584c51737bd664387 Signed-off-by: Markus Armbruster Signed-off-by: Daniel P. Berrangé --- authz/listfile.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/authz/listfile.c b/authz/listfile.c index 24feac35abe8..1421e674a466 100644 --- a/authz/listfile.c +++ b/authz/listfile.c @@ -128,6 +128,9 @@ qauthz_list_file_complete(UserCreatable *uc, Error **errp) } fauthz->list = qauthz_list_file_load(fauthz, errp); + if (!fauthz->list) { + return; + } if (!fauthz->refresh) { return; From 8e26ae7bb58d10c04599eabd265217da050514a4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Nov 2020 07:23:58 +0100 Subject: [PATCH 2/4] authz-list-file: Improve an error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When qauthz_list_file_load() rejects JSON values other than JSON object with a rather confusing error message: $ echo 1 | qemu-system-x86_64 -nodefaults -S -display none -object authz-list-file,id=authz0,filename=/dev/stdin qemu-system-x86_64: -object authz-list-file,id=authz0,filename=/dev/stdin: Invalid parameter type for 'obj', expected: dict Improve to qemu-system-x86_64: -object authz-list-file,id=authz0,filename=/dev/stdin: File '/dev/stdin' must contain a JSON object Signed-off-by: Markus Armbruster Signed-off-by: Daniel P. Berrangé --- authz/listfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/authz/listfile.c b/authz/listfile.c index 1421e674a466..da3a0e69a2ed 100644 --- a/authz/listfile.c +++ b/authz/listfile.c @@ -73,7 +73,8 @@ qauthz_list_file_load(QAuthZListFile *fauthz, Error **errp) pdict = qobject_to(QDict, obj); if (!pdict) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "obj", "dict"); + error_setg(errp, "File '%s' must contain a JSON object", + fauthz->filename); goto cleanup; } From 3428455df9302b2b924e380cb90a77ca1ce5001e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 17 Nov 2020 17:30:44 +0100 Subject: [PATCH 3/4] authz-pam: Check that 'service' property is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the 'service' property is not set, we'll call pam_start() with a NULL pointer for the service name. This fails and leaves a message like this in the syslog: qemu-storage-daemon[294015]: PAM pam_start: invalid argument: service == NULL Make specifying the property mandatory and catch the error already during the creation of the object. Signed-off-by: Kevin Wolf Signed-off-by: Daniel P. Berrangé --- authz/pamacct.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/authz/pamacct.c b/authz/pamacct.c index e67195f7be0f..c862d9ff39b2 100644 --- a/authz/pamacct.c +++ b/authz/pamacct.c @@ -84,6 +84,12 @@ qauthz_pam_prop_get_service(Object *obj, static void qauthz_pam_complete(UserCreatable *uc, Error **errp) { + QAuthZPAM *pauthz = QAUTHZ_PAM(uc); + + if (!pauthz->service) { + error_setg(errp, "The 'service' property must be set"); + return; + } } From c2aa8a3d7e5ce57fa3df310c9b7ca48fcbf9d4ad Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 17 Nov 2020 17:30:45 +0100 Subject: [PATCH 4/4] authz-simple: Check that 'identity' property is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the 'identify' property is not set, we'll pass a NULL pointer to g_str_equal() and crash. Catch the error condition during the creation of the object. Signed-off-by: Kevin Wolf Signed-off-by: Daniel P. Berrangé --- authz/simple.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/authz/simple.c b/authz/simple.c index 18db0355f408..0597dcd8ea5a 100644 --- a/authz/simple.c +++ b/authz/simple.c @@ -65,11 +65,25 @@ qauthz_simple_finalize(Object *obj) } +static void +qauthz_simple_complete(UserCreatable *uc, Error **errp) +{ + QAuthZSimple *sauthz = QAUTHZ_SIMPLE(uc); + + if (!sauthz->identity) { + error_setg(errp, "The 'identity' property must be set"); + return; + } +} + + static void qauthz_simple_class_init(ObjectClass *oc, void *data) { QAuthZClass *authz = QAUTHZ_CLASS(oc); + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); + ucc->complete = qauthz_simple_complete; authz->is_allowed = qauthz_simple_is_allowed; object_class_property_add_str(oc, "identity",