* [Qemu-devel] [PATCH 0/2] curses Coverity fixes @ 2016-08-11 14:23 Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Peter Maydell @ 2016-08-11 14:23 UTC (permalink / raw) To: qemu-devel; +Cc: patches, Gerd Hoffmann This patchset fixes a couple of issues found by Coverity in the curses ui frontend. thanks -- PMM Peter Maydell (2): ui/curses.c: Ensure we don't read off the end of curses2qemu array ui/curses.c: Clean up nextchr logic ui/curses.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array 2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell @ 2016-08-11 14:23 ` Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic Peter Maydell 2016-09-07 11:39 ` [Qemu-devel] [PATCH 0/2] curses Coverity fixes Gerd Hoffmann 2 siblings, 0 replies; 4+ messages in thread From: Peter Maydell @ 2016-08-11 14:23 UTC (permalink / raw) To: qemu-devel; +Cc: patches, Gerd Hoffmann Coverity spots that there is no bounds check before we access the curses2qemu[] array. Add one, bringing this code path into line with the one that looks up entries in curses2keysym[]. In theory getch() shouldn't return out of range keycodes, but it's better not to assume this. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- ui/curses.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/curses.c b/ui/curses.c index b475589..f1f886c 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -317,7 +317,10 @@ static void curses_refresh(DisplayChangeListener *dcl) qemu_input_event_send_key_delay(0); } } else { - keysym = curses2qemu[chr]; + keysym = -1; + if (chr < CURSES_KEYS) { + keysym = curses2qemu[chr]; + } if (keysym == -1) keysym = chr; -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic 2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell @ 2016-08-11 14:23 ` Peter Maydell 2016-09-07 11:39 ` [Qemu-devel] [PATCH 0/2] curses Coverity fixes Gerd Hoffmann 2 siblings, 0 replies; 4+ messages in thread From: Peter Maydell @ 2016-08-11 14:23 UTC (permalink / raw) To: qemu-devel; +Cc: patches, Gerd Hoffmann Coverity identifies that at the top of the while(1) loop in curses_refresh() the variable nextchr is always ERR, and so the else case of the first if() is dead code. Remove this dead code, and narrow the scope of the nextchr variable to the place where it's used. (This confused logic has been present since the curses code was added to QEMU in 2008.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- ui/curses.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/ui/curses.c b/ui/curses.c index f1f886c..d06f724 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -181,7 +181,7 @@ static kbd_layout_t *kbd_layout = NULL; static void curses_refresh(DisplayChangeListener *dcl) { - int chr, nextchr, keysym, keycode, keycode_alt; + int chr, keysym, keycode, keycode_alt; curses_winch_check(); @@ -195,15 +195,9 @@ static void curses_refresh(DisplayChangeListener *dcl) graphic_hw_text_update(NULL, screen); - nextchr = ERR; while (1) { /* while there are any pending key strokes to process */ - if (nextchr == ERR) - chr = getch(); - else { - chr = nextchr; - nextchr = ERR; - } + chr = getch(); if (chr == ERR) break; @@ -224,13 +218,12 @@ static void curses_refresh(DisplayChangeListener *dcl) /* alt key */ if (keycode == 1) { - nextchr = getch(); + int nextchr = getch(); if (nextchr != ERR) { chr = nextchr; keycode_alt = ALT; - keycode = curses2keycode[nextchr]; - nextchr = ERR; + keycode = curses2keycode[chr]; if (keycode != -1) { keycode |= ALT; -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] curses Coverity fixes 2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic Peter Maydell @ 2016-09-07 11:39 ` Gerd Hoffmann 2 siblings, 0 replies; 4+ messages in thread From: Gerd Hoffmann @ 2016-09-07 11:39 UTC (permalink / raw) To: Peter Maydell; +Cc: qemu-devel, patches On Do, 2016-08-11 at 15:23 +0100, Peter Maydell wrote: > This patchset fixes a couple of issues found by Coverity > in the curses ui frontend. Added to ui queue. thanks, Gerd ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-07 11:40 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell 2016-08-11 14:23 ` [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic Peter Maydell 2016-09-07 11:39 ` [Qemu-devel] [PATCH 0/2] curses Coverity fixes Gerd Hoffmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).