* [PATCH 0/2] authz-list-file: Error handling fixes @ 2020-11-13 6:23 Markus Armbruster 2020-11-13 6:23 ` [PATCH 1/2] authz-list-file: Fix file read error handling Markus Armbruster 2020-11-13 6:23 ` [PATCH 2/2] authz-list-file: Improve an error message Markus Armbruster 0 siblings, 2 replies; 6+ messages in thread From: Markus Armbruster @ 2020-11-13 6:23 UTC (permalink / raw) To: qemu-devel; +Cc: berrange Possibly 5.2 material, because a silly mistake in use of object-add can crash QEMU. Markus Armbruster (2): authz-list-file: Fix file read error handling authz-list-file: Improve an error message authz/listfile.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] authz-list-file: Fix file read error handling 2020-11-13 6:23 [PATCH 0/2] authz-list-file: Error handling fixes Markus Armbruster @ 2020-11-13 6:23 ` Markus Armbruster 2020-11-13 9:37 ` Daniel P. Berrangé 2020-11-13 6:23 ` [PATCH 2/2] authz-list-file: Improve an error message Markus Armbruster 1 sibling, 1 reply; 6+ messages in thread From: Markus Armbruster @ 2020-11-13 6:23 UTC (permalink / raw) To: qemu-devel; +Cc: berrange 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 <armbru@redhat.com> --- authz/listfile.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/authz/listfile.c b/authz/listfile.c index 24feac35ab..1421e674a4 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; -- 2.26.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] authz-list-file: Fix file read error handling 2020-11-13 6:23 ` [PATCH 1/2] authz-list-file: Fix file read error handling Markus Armbruster @ 2020-11-13 9:37 ` Daniel P. Berrangé 0 siblings, 0 replies; 6+ messages in thread From: Daniel P. Berrangé @ 2020-11-13 9:37 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel On Fri, Nov 13, 2020 at 07:23:57AM +0100, Markus Armbruster wrote: > 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 <armbru@redhat.com> > --- > authz/listfile.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] authz-list-file: Improve an error message 2020-11-13 6:23 [PATCH 0/2] authz-list-file: Error handling fixes Markus Armbruster 2020-11-13 6:23 ` [PATCH 1/2] authz-list-file: Fix file read error handling Markus Armbruster @ 2020-11-13 6:23 ` Markus Armbruster 2020-11-13 9:36 ` Daniel P. Berrangé 1 sibling, 1 reply; 6+ messages in thread From: Markus Armbruster @ 2020-11-13 6:23 UTC (permalink / raw) To: qemu-devel; +Cc: berrange 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 <armbru@redhat.com> --- authz/listfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/authz/listfile.c b/authz/listfile.c index 1421e674a4..da3a0e69a2 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; } -- 2.26.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] authz-list-file: Improve an error message 2020-11-13 6:23 ` [PATCH 2/2] authz-list-file: Improve an error message Markus Armbruster @ 2020-11-13 9:36 ` Daniel P. Berrangé 2020-11-13 12:54 ` Markus Armbruster 0 siblings, 1 reply; 6+ messages in thread From: Daniel P. Berrangé @ 2020-11-13 9:36 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel On Fri, Nov 13, 2020 at 07:23:58AM +0100, Markus Armbruster wrote: > 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 <armbru@redhat.com> > --- > authz/listfile.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > diff --git a/authz/listfile.c b/authz/listfile.c > index 1421e674a4..da3a0e69a2 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); This code pattern was copied from other places in QEMU which use the same QERR_INVALID_PARAMETER_TYPE. There are another 10 or so examples of this error message pattern. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] authz-list-file: Improve an error message 2020-11-13 9:36 ` Daniel P. Berrangé @ 2020-11-13 12:54 ` Markus Armbruster 0 siblings, 0 replies; 6+ messages in thread From: Markus Armbruster @ 2020-11-13 12:54 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel Daniel P. Berrangé <berrange@redhat.com> writes: > On Fri, Nov 13, 2020 at 07:23:58AM +0100, Markus Armbruster wrote: >> 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 <armbru@redhat.com> >> --- >> authz/listfile.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Thanks! >> diff --git a/authz/listfile.c b/authz/listfile.c >> index 1421e674a4..da3a0e69a2 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); > > This code pattern was copied from other places in QEMU which use the > same QERR_INVALID_PARAMETER_TYPE. There are another 10 or so examples > of this error message pattern. Yes. My "[PATCH 00/10] Chipping away at qerror.h" fixes other patterns, but not this one. I intend to chip some more, as time permits. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-13 12:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-13 6:23 [PATCH 0/2] authz-list-file: Error handling fixes Markus Armbruster 2020-11-13 6:23 ` [PATCH 1/2] authz-list-file: Fix file read error handling Markus Armbruster 2020-11-13 9:37 ` Daniel P. Berrangé 2020-11-13 6:23 ` [PATCH 2/2] authz-list-file: Improve an error message Markus Armbruster 2020-11-13 9:36 ` Daniel P. Berrangé 2020-11-13 12:54 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.