| Message ID | 20190725032321.12721-8-alxndr@bu.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add virtual device fuzzing support | expand |
On 25/07/2019 05.23, Oleinik, Alexander wrote: > libqtest directly invokes the qtest client and exposes a function to > accept responses. > > Signed-off-by: Alexander Oleinik <alxndr@bu.edu> > --- > tests/libqtest.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++- > tests/libqtest.h | 6 ++++++ > 2 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 3c5c3f49d8..a68a7287cb 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -30,12 +30,18 @@ > #include "qapi/qmp/qjson.h" > #include "qapi/qmp/qlist.h" > #include "qapi/qmp/qstring.h" > +#ifdef CONFIG_FUZZ > +#include "sysemu/qtest.h" > +#endif > > #define MAX_IRQ 256 > #define SOCKET_TIMEOUT 50 > #define SOCKET_MAX_FDS 16 > > QTestState *global_qtest; > +#ifdef CONFIG_FUZZ > +static GString *recv_str; > +#endif > > struct QTestState > { > @@ -316,6 +322,20 @@ QTestState *qtest_initf(const char *fmt, ...) > va_end(ap); > return s; > } > +#ifdef CONFIG_FUZZ > +QTestState *qtest_init_fuzz(const char *extra_args, int *sock_fd) > +{ > + QTestState *qts; > + qts = g_new(QTestState, 1); > + qts->wstatus = 0; > + for (int i = 0; i < MAX_IRQ; i++) { > + qts->irq_level[i] = false; > + } > + qts->big_endian = qtest_query_target_endianness(qts); > + > + return qts; > +} > +#endif > > QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) > { > @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt, va_list ap) > { > gchar *str = g_strdup_vprintf(fmt, ap); > size_t size = strlen(str); > +#ifdef CONFIG_FUZZ > + // Directly call qtest_process_inbuf in the qtest server > + GString *gstr = g_string_new_len(str, size); > + /* printf(">>> %s",gstr->str); */ Please check your patches with scripts/checkpatch.pl - e.g. don't use TABs for indentation like in the above line, don't use //-comments, etc. > + qtest_server_recv(gstr); > + g_string_free(gstr, true); > + g_free(str); > +#else > > socket_send(fd, str, size); > g_free(str); > +#endif > } > > static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...) > @@ -433,6 +462,12 @@ static GString *qtest_recv_line(QTestState *s) > size_t offset; > char *eol; > > +#ifdef CONFIG_FUZZ > + eol = strchr(recv_str->str, '\n'); > + offset = eol - recv_str->str; > + line = g_string_new_len(recv_str->str, offset); > + g_string_erase(recv_str, 0, offset + 1); > +#else > while ((eol = strchr(s->rx->str, '\n')) == NULL) { > ssize_t len; > char buffer[1024]; > @@ -453,7 +488,7 @@ static GString *qtest_recv_line(QTestState *s) > offset = eol - s->rx->str; > line = g_string_new_len(s->rx->str, offset); > g_string_erase(s->rx, 0, offset + 1); > - > +#endif > return line; > } > > @@ -797,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...) > > const char *qtest_get_arch(void) > { > +#ifdef CONFIG_FUZZ > + return "i386"; > +#endif Hard-coding "i386" is quite ugly ... it's ok for an RFC patch, but I think this should be fixed in the final version of the patches. Maybe you could use TARGET_NAME instead? > const char *qemu = qtest_qemu_binary(); > const char *end = strrchr(qemu, '/'); > > @@ -1339,3 +1377,16 @@ void qmp_assert_error_class(QDict *rsp, const char *class) > > qobject_unref(rsp); > } > +#ifdef CONFIG_FUZZ > +void qtest_clear_rxbuf(QTestState *s){ For functions, the curly brace should start on a new line. > + g_string_set_size(recv_str,0); > +} Thomas
On 25/07/19 11:04, Thomas Huth wrote: >> @@ -797,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...) >> >> const char *qtest_get_arch(void) >> { >> +#ifdef CONFIG_FUZZ >> + return "i386"; >> +#endif > > Hard-coding "i386" is quite ugly ... it's ok for an RFC patch, but I > think this should be fixed in the final version of the patches. Maybe > you could use TARGET_NAME instead? Yes, TARGET_NAME is the one. Also I would just split the file in two: the common bits that are used for both libqtest and fuzz in one file, so the libqtest and fuzz "drivers" can be in completely separate file without #ifdefs. Paolo > >> const char *qemu = qtest_qemu_binary(); >> const char *end = strrchr(qemu, '/'); >> >> @@ -1339,3 +1377,16 @@ void qmp_assert_error_class(QDict *rsp, const char *class) >> >> qobject_unref(rsp); >> } >> +#ifdef CONFIG_FUZZ >> +void qtest_clear_rxbuf(QTestState *s){ > > For functions, the curly brace should start on a new line. > >> + g_string_set_size(recv_str,0); >> +} > > Thomas >
On Thu, Jul 25, 2019 at 11:04:11AM +0200, Thomas Huth wrote: > On 25/07/2019 05.23, Oleinik, Alexander wrote: > > @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt, va_list ap) > > { > > gchar *str = g_strdup_vprintf(fmt, ap); > > size_t size = strlen(str); > > +#ifdef CONFIG_FUZZ > > + // Directly call qtest_process_inbuf in the qtest server > > + GString *gstr = g_string_new_len(str, size); > > + /* printf(">>> %s",gstr->str); */ > > Please check your patches with scripts/checkpatch.pl - e.g. don't use > TABs for indentation like in the above line, don't use //-comments, etc. You can set up a git-hook with checkpatch.pl to scan code automatically before each commit: http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html Stefan
On Thu, Jul 25, 2019 at 03:23:49AM +0000, Oleinik, Alexander wrote: > @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt, va_list ap) > { > gchar *str = g_strdup_vprintf(fmt, ap); > size_t size = strlen(str); > +#ifdef CONFIG_FUZZ > + // Directly call qtest_process_inbuf in the qtest server > + GString *gstr = g_string_new_len(str, size); > + /* printf(">>> %s",gstr->str); */ > + qtest_server_recv(gstr); > + g_string_free(gstr, true); > + g_free(str); > +#else > > socket_send(fd, str, size); > g_free(str); > +#endif > } This should use indirection: a function pointer to dispatch to either the socket or the internal qtest_process_inbuf() call. With a bit of refactoring you can eliminate the #ifdefs and treat the socket fd as one backend and direct invocation as another backend.
On 26/07/19 14:56, Stefan Hajnoczi wrote: > This should use indirection: a function pointer to dispatch to either > the socket or the internal qtest_process_inbuf() call. > > With a bit of refactoring you can eliminate the #ifdefs and treat the > socket fd as one backend and direct invocation as another backend. My suggestion was a bit different (two files), but this also works. In fact it can also be combined to have three files: - one defining libqtest's qtest_init and associated struct of function pointers - one defining the fuzzer's qtest_init and associated struct of function pointers - one with the remaining libqtest code, modified to use the struct of function pointers for everything that you're #ifdef-ing here, and a function qtest_client_init that receives the struct of function pointers and stores them in QTestState. The two qtest_init implementations in the other files just call qtest_client_init. Paolo
diff --git a/tests/libqtest.c b/tests/libqtest.c index 3c5c3f49d8..a68a7287cb 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -30,12 +30,18 @@ #include "qapi/qmp/qjson.h" #include "qapi/qmp/qlist.h" #include "qapi/qmp/qstring.h" +#ifdef CONFIG_FUZZ +#include "sysemu/qtest.h" +#endif #define MAX_IRQ 256 #define SOCKET_TIMEOUT 50 #define SOCKET_MAX_FDS 16 QTestState *global_qtest; +#ifdef CONFIG_FUZZ +static GString *recv_str; +#endif struct QTestState { @@ -316,6 +322,20 @@ QTestState *qtest_initf(const char *fmt, ...) va_end(ap); return s; } +#ifdef CONFIG_FUZZ +QTestState *qtest_init_fuzz(const char *extra_args, int *sock_fd) +{ + QTestState *qts; + qts = g_new(QTestState, 1); + qts->wstatus = 0; + for (int i = 0; i < MAX_IRQ; i++) { + qts->irq_level[i] = false; + } + qts->big_endian = qtest_query_target_endianness(qts); + + return qts; +} +#endif QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) { @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt, va_list ap) { gchar *str = g_strdup_vprintf(fmt, ap); size_t size = strlen(str); +#ifdef CONFIG_FUZZ + // Directly call qtest_process_inbuf in the qtest server + GString *gstr = g_string_new_len(str, size); + /* printf(">>> %s",gstr->str); */ + qtest_server_recv(gstr); + g_string_free(gstr, true); + g_free(str); +#else socket_send(fd, str, size); g_free(str); +#endif } static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...) @@ -433,6 +462,12 @@ static GString *qtest_recv_line(QTestState *s) size_t offset; char *eol; +#ifdef CONFIG_FUZZ + eol = strchr(recv_str->str, '\n'); + offset = eol - recv_str->str; + line = g_string_new_len(recv_str->str, offset); + g_string_erase(recv_str, 0, offset + 1); +#else while ((eol = strchr(s->rx->str, '\n')) == NULL) { ssize_t len; char buffer[1024]; @@ -453,7 +488,7 @@ static GString *qtest_recv_line(QTestState *s) offset = eol - s->rx->str; line = g_string_new_len(s->rx->str, offset); g_string_erase(s->rx, 0, offset + 1); - +#endif return line; } @@ -797,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...) const char *qtest_get_arch(void) { +#ifdef CONFIG_FUZZ + return "i386"; +#endif const char *qemu = qtest_qemu_binary(); const char *end = strrchr(qemu, '/'); @@ -1339,3 +1377,16 @@ void qmp_assert_error_class(QDict *rsp, const char *class) qobject_unref(rsp); } +#ifdef CONFIG_FUZZ +void qtest_clear_rxbuf(QTestState *s){ + g_string_set_size(recv_str,0); +} + +void qtest_client_recv(const char *str, size_t len) +{ + if(!recv_str) + recv_str = g_string_new(NULL); + g_string_append_len(recv_str, str, len); + return; +} +#endif diff --git a/tests/libqtest.h b/tests/libqtest.h index cadf1d4a03..dca8f2c2f2 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -1001,4 +1001,10 @@ void qmp_assert_error_class(QDict *rsp, const char *class); */ bool qtest_probe_child(QTestState *s); +#ifdef CONFIG_FUZZ +QTestState *qtest_init_fuzz(const char *extra_args, int *sock_fd); +void qtest_clear_rxbuf(QTestState *s); +void qtest_client_recv(const char *str, size_t len); +#endif + #endif
libqtest directly invokes the qtest client and exposes a function to accept responses. Signed-off-by: Alexander Oleinik <alxndr@bu.edu> --- tests/libqtest.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++- tests/libqtest.h | 6 ++++++ 2 files changed, 58 insertions(+), 1 deletion(-)