[v1] ui/sdl2: fix segment fault caused by null pointer dereference

[PATCH] ui/sdl2: fix segment fault caused by null pointer dereference

Posted by Changbin Du 5 years, 6 months ago

I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
ssh forwarding even the window has been crated already. I am not sure
whether this is a bug of SDL, but we'd better check it carefully.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 ui/sdl2.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 3c9424eb42..7c9c93b951 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -332,6 +332,9 @@ static void handle_keydown(SDL_Event *ev)
     int gui_key_modifier_pressed = get_mod_state();
     int gui_keysym = 0;
 
+    if (!scon)
+        return;
+
     if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
         switch (ev->key.keysym.scancode) {
         case SDL_SCANCODE_2:
@@ -412,6 +415,9 @@ static void handle_keyup(SDL_Event *ev)
 {
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!sconf)
+        return;
+
     scon->ignore_hotkeys = false;
     sdl2_process_key(scon, &ev->key);
 }
@@ -421,6 +427,9 @@ static void handle_textinput(SDL_Event *ev)
     struct sdl2_console *scon = get_scon_from_window(ev->text.windowID);
     QemuConsole *con = scon ? scon->dcl.con : NULL;
 
+    if (!con)
+        return;
+
     if (qemu_console_is_graphic(con)) {
         return;
     }
-- 
2.20.1

Re: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference

Posted by no-reply@patchew.org 5 years, 6 months ago

Patchew URL: https://patchew.org/QEMU/20200427121823.8094-1-changbin.du@gmail.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  BUILD   pc-bios/optionrom/linuxboot_dma.img
  BUILD   pc-bios/optionrom/linuxboot_dma.raw
  SIGN    pc-bios/optionrom/linuxboot_dma.bin
/tmp/qemu-test/src/ui/sdl2.c:418:10: error: use of undeclared identifier 'sconf'
    if (!sconf)
         ^
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: ui/sdl2.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=4fa702e41a1541379c097297271712f7', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-xsh4udx7/src/docker-src.2020-04-27-08.25.03.16219:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=4fa702e41a1541379c097297271712f7
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-xsh4udx7/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m22.841s
user    0m9.190s


The full log is available at
http://patchew.org/logs/20200427121823.8094-1-changbin.du@gmail.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference

Posted by no-reply@patchew.org 5 years, 6 months ago

Patchew URL: https://patchew.org/QEMU/20200427121823.8094-1-changbin.du@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference
Message-id: 20200427121823.8094-1-changbin.du@gmail.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cc4f090 ui/sdl2: fix segment fault caused by null pointer dereference

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#32: FILE: ui/sdl2.c:418:
+    if (!sconf)
[...]

ERROR: braces {} are necessary for all arms of this statement
#42: FILE: ui/sdl2.c:430:
+    if (!con)
[...]

total: 2 errors, 0 warnings, 27 lines checked

Commit cc4f090c07bc (ui/sdl2: fix segment fault caused by null pointer dereference) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200427121823.8094-1-changbin.du@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference

Posted by Peter Maydell 5 years, 6 months ago

On Mon, 27 Apr 2020 at 13:19, Changbin Du <changbin.du@gmail.com> wrote:
>
> I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
> ssh forwarding even the window has been crated already. I am not sure
> whether this is a bug of SDL, but we'd better check it carefully.
>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
>  ui/sdl2.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 3c9424eb42..7c9c93b951 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -332,6 +332,9 @@ static void handle_keydown(SDL_Event *ev)
>      int gui_key_modifier_pressed = get_mod_state();
>      int gui_keysym = 0;
>
> +    if (!scon)
> +        return;
> +
>      if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
>          switch (ev->key.keysym.scancode) {
>          case SDL_SCANCODE_2:
> @@ -412,6 +415,9 @@ static void handle_keyup(SDL_Event *ev)
>  {
>      struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
>
> +    if (!sconf)
> +        return;

It's generally a good idea to make sure your patch at least compiles
before sending it :-)

QEMU coding style demands {} on all if statements.

thanks
-- PMM

Re: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference

Posted by Changbin Du 5 years, 6 months ago

On Mon, Apr 27, 2020 at 02:11:59PM +0100, Peter Maydell wrote:
> On Mon, 27 Apr 2020 at 13:19, Changbin Du <changbin.du@gmail.com> wrote:
> >
> > I found SDL_GetWindowFromID() sometimes return NULL when I start qemu via
> > ssh forwarding even the window has been crated already. I am not sure
> > whether this is a bug of SDL, but we'd better check it carefully.
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> >  ui/sdl2.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index 3c9424eb42..7c9c93b951 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -332,6 +332,9 @@ static void handle_keydown(SDL_Event *ev)
> >      int gui_key_modifier_pressed = get_mod_state();
> >      int gui_keysym = 0;
> >
> > +    if (!scon)
> > +        return;
> > +
> >      if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
> >          switch (ev->key.keysym.scancode) {
> >          case SDL_SCANCODE_2:
> > @@ -412,6 +415,9 @@ static void handle_keyup(SDL_Event *ev)
> >  {
> >      struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
> >
> > +    if (!sconf)
> > +        return;
> 
> It's generally a good idea to make sure your patch at least compiles
> before sending it :-)
>
sorry for this. I don't know why my make didn't recompile it after
changing.

> QEMU coding style demands {} on all if statements.
> 
sure.

> thanks
> -- PMM

-- 
Cheers,
Changbin Du

Re: [PATCH] ui/sdl2: fix segment fault caused by null pointer dereference

Posted by no-reply@patchew.org 5 years, 6 months ago

Patchew URL: https://patchew.org/QEMU/20200427121823.8094-1-changbin.du@gmail.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  SIGN    pc-bios/optionrom/pvh.bin
  LINK    qemu-io.exe
/tmp/qemu-test/src/ui/sdl2.c: In function 'handle_keyup':
/tmp/qemu-test/src/ui/sdl2.c:418:10: error: 'sconf' undeclared (first use in this function); did you mean 'scon'?
     if (!sconf)
          ^~~~~
          scon
/tmp/qemu-test/src/ui/sdl2.c:418:10: note: each undeclared identifier is reported only once for each function it appears in
make: *** [/tmp/qemu-test/src/rules.mak:69: ui/sdl2.o] Error 1
make: *** Waiting for unfinished jobs....
  LINK    qemu-edid.exe
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=874a446966454b89aa835e525ea0e720', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-z9iu_bje/src/docker-src.2020-04-27-08.40.34.32723:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=874a446966454b89aa835e525ea0e720
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-z9iu_bje/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m58.815s
user    0m8.637s


The full log is available at
http://patchew.org/logs/20200427121823.8094-1-changbin.du@gmail.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com