diff mbox series

[v2] ui/console: Precautionary glBindTexture and surface->texture validation in surface_gl_update_texture

Message ID 20190507054914.25261-1-marcel.apfelbaum@gmail.com
State New
Headers show
Series [v2] ui/console: Precautionary glBindTexture and surface->texture validation in surface_gl_update_texture | expand

Commit Message

Marcel Apfelbaum May 7, 2019, 5:49 a.m. UTC 
From: HQM <hqm03ster@gmail.com>

In a GVT-g setup with dmabuf and GTK GUI, the current 2D texture at
surface_gl_update_texture is not necessarily
surface->texture. Adding a glBindTexture fixes related crashes and
artifacts, and is generally more secure.

Signed-off-by: HQM <hqm03ster@gmail.com>
Tested-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com>
[fixed malformed patch, rebase to master]
Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
---

v2:
 - fixed malformed patch
 - rebased to master

 ui/console-gl.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Philippe Mathieu-Daudé May 7, 2019, 6:25 a.m. UTC | #1 
Hi Marcel,

On 5/7/19 7:49 AM, Marcel Apfelbaum wrote:
> From: HQM <hqm03ster@gmail.com>
> 
> In a GVT-g setup with dmabuf and GTK GUI, the current 2D texture at
> surface_gl_update_texture is not necessarily
> surface->texture. Adding a glBindTexture fixes related crashes and
> artifacts, and is generally more secure.
> 
> Signed-off-by: HQM <hqm03ster@gmail.com>

This looks like an acronym, per
https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line

"Patch emails must include a Signed-off-by: line [...] Please use your
real name to sign a patch (not an alias or acronym)."

> Tested-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com>
> [fixed malformed patch, rebase to master]
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> ---
> 
> v2:
>  - fixed malformed patch
>  - rebased to master
> 
>  ui/console-gl.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/ui/console-gl.c b/ui/console-gl.c
> index a56e1cd8eb..c1cb3bd673 100644
> --- a/ui/console-gl.c
> +++ b/ui/console-gl.c
> @@ -92,13 +92,17 @@ void surface_gl_update_texture(QemuGLShader *gls,
>  
>      assert(gls);
>  
> -    glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> -                  surface_stride(surface) / surface_bytes_per_pixel(surface));
> -    glTexSubImage2D(GL_TEXTURE_2D, 0,
> -                    x, y, w, h,
> -                    surface->glformat, surface->gltype,
> -                    data + surface_stride(surface) * y
> -                    + surface_bytes_per_pixel(surface) * x);
> +    if (surface->texture) {
> +        glBindTexture(GL_TEXTURE_2D, surface->texture);
> +        glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> +                      surface_stride(surface)
> +                      / surface_bytes_per_pixel(surface));
> +        glTexSubImage2D(GL_TEXTURE_2D, 0,
> +                        x, y, w, h,
> +                        surface->glformat, surface->gltype,
> +                        data + surface_stride(surface) * y
> +                        + surface_bytes_per_pixel(surface) * x);
> +    }
>  }
>  
>  void surface_gl_render_texture(QemuGLShader *gls,
>
Hou Qiming May 7, 2019, 6:49 a.m. UTC | #2 
My real name is "HOU Qiming". @Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
can you incorporate that in your v2 patch? Thanks!

Qiming

On Tue, May 7, 2019 at 2:25 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi Marcel,
>
> On 5/7/19 7:49 AM, Marcel Apfelbaum wrote:
> > From: HQM <hqm03ster@gmail.com>
> >
> > In a GVT-g setup with dmabuf and GTK GUI, the current 2D texture at
> > surface_gl_update_texture is not necessarily
> > surface->texture. Adding a glBindTexture fixes related crashes and
> > artifacts, and is generally more secure.
> >
> > Signed-off-by: HQM <hqm03ster@gmail.com>
>
> This looks like an acronym, per
>
> https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
>
> "Patch emails must include a Signed-off-by: line [...] Please use your
> real name to sign a patch (not an alias or acronym)."
>
> > Tested-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com>
> > [fixed malformed patch, rebase to master]
> > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > ---
> >
> > v2:
> >  - fixed malformed patch
> >  - rebased to master
> >
> >  ui/console-gl.c | 18 +++++++++++-------
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/ui/console-gl.c b/ui/console-gl.c
> > index a56e1cd8eb..c1cb3bd673 100644
> > --- a/ui/console-gl.c
> > +++ b/ui/console-gl.c
> > @@ -92,13 +92,17 @@ void surface_gl_update_texture(QemuGLShader *gls,
> >
> >      assert(gls);
> >
> > -    glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> > -                  surface_stride(surface) /
> surface_bytes_per_pixel(surface));
> > -    glTexSubImage2D(GL_TEXTURE_2D, 0,
> > -                    x, y, w, h,
> > -                    surface->glformat, surface->gltype,
> > -                    data + surface_stride(surface) * y
> > -                    + surface_bytes_per_pixel(surface) * x);
> > +    if (surface->texture) {
> > +        glBindTexture(GL_TEXTURE_2D, surface->texture);
> > +        glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> > +                      surface_stride(surface)
> > +                      / surface_bytes_per_pixel(surface));
> > +        glTexSubImage2D(GL_TEXTURE_2D, 0,
> > +                        x, y, w, h,
> > +                        surface->glformat, surface->gltype,
> > +                        data + surface_stride(surface) * y
> > +                        + surface_bytes_per_pixel(surface) * x);
> > +    }
> >  }
> >
> >  void surface_gl_render_texture(QemuGLShader *gls,
> >
>
Philippe Mathieu-Daudé May 7, 2019, 7:44 a.m. UTC | #3 
On 5/7/19 8:49 AM, Hou Qiming wrote:
> My real name is "HOU Qiming". @Marcel Apfelbaum
> <mailto:marcel.apfelbaum@gmail.com> can you incorporate that in your v2
> patch? Thanks!

Thanks a lot Qiming :)

> On Tue, May 7, 2019 at 2:25 PM Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>> wrote:
> 
>     Hi Marcel,
> 
>     On 5/7/19 7:49 AM, Marcel Apfelbaum wrote:
>     > From: HQM <hqm03ster@gmail.com <mailto:hqm03ster@gmail.com>>
>     >
>     > In a GVT-g setup with dmabuf and GTK GUI, the current 2D texture at
>     > surface_gl_update_texture is not necessarily
>     > surface->texture. Adding a glBindTexture fixes related crashes and
>     > artifacts, and is generally more secure.
>     >
>     > Signed-off-by: HQM <hqm03ster@gmail.com <mailto:hqm03ster@gmail.com>>
> 
>     This looks like an acronym, per
>     https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
> 
>     "Patch emails must include a Signed-off-by: line [...] Please use your
>     real name to sign a patch (not an alias or acronym)."
> 
>     > Tested-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com
>     <mailto:marcel.apfelbaum@gmail.com>>
>     > [fixed malformed patch, rebase to master]
>     > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com
>     <mailto:marcel.apfelbaum@gmail.com>>
>     > ---
>     >
>     > v2:
>     >  - fixed malformed patch
>     >  - rebased to master
>     >
>     >  ui/console-gl.c | 18 +++++++++++-------
>     >  1 file changed, 11 insertions(+), 7 deletions(-)
>     >
>     > diff --git a/ui/console-gl.c b/ui/console-gl.c
>     > index a56e1cd8eb..c1cb3bd673 100644
>     > --- a/ui/console-gl.c
>     > +++ b/ui/console-gl.c
>     > @@ -92,13 +92,17 @@ void surface_gl_update_texture(QemuGLShader *gls,
>     > 
>     >      assert(gls);
>     > 
>     > -    glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
>     > -                  surface_stride(surface) /
>     surface_bytes_per_pixel(surface));
>     > -    glTexSubImage2D(GL_TEXTURE_2D, 0,
>     > -                    x, y, w, h,
>     > -                    surface->glformat, surface->gltype,
>     > -                    data + surface_stride(surface) * y
>     > -                    + surface_bytes_per_pixel(surface) * x);
>     > +    if (surface->texture) {
>     > +        glBindTexture(GL_TEXTURE_2D, surface->texture);
>     > +        glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
>     > +                      surface_stride(surface)
>     > +                      / surface_bytes_per_pixel(surface));
>     > +        glTexSubImage2D(GL_TEXTURE_2D, 0,
>     > +                        x, y, w, h,
>     > +                        surface->glformat, surface->gltype,
>     > +                        data + surface_stride(surface) * y
>     > +                        + surface_bytes_per_pixel(surface) * x);
>     > +    }
>     >  }
>     > 
>     >  void surface_gl_render_texture(QemuGLShader *gls,
>     >
>
Marcel Apfelbaum May 7, 2019, 8 a.m. UTC | #4 
On 5/7/19 9:49 AM, Hou Qiming wrote:
> My real name is "HOU Qiming". @Marcel Apfelbaum 
> <mailto:marcel.apfelbaum@gmail.com> can you incorporate that in your 
> v2 patch? Thanks!
>

Sure thing,

Thanks,
Marcel

> Qiming
>
> On Tue, May 7, 2019 at 2:25 PM Philippe Mathieu-Daudé 
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
>
>     Hi Marcel,
>
>     On 5/7/19 7:49 AM, Marcel Apfelbaum wrote:
>     > From: HQM <hqm03ster@gmail.com <mailto:hqm03ster@gmail.com>>
>     >
>     > In a GVT-g setup with dmabuf and GTK GUI, the current 2D texture at
>     > surface_gl_update_texture is not necessarily
>     > surface->texture. Adding a glBindTexture fixes related crashes and
>     > artifacts, and is generally more secure.
>     >
>     > Signed-off-by: HQM <hqm03ster@gmail.com
>     <mailto:hqm03ster@gmail.com>>
>
>     This looks like an acronym, per
>     https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
>
>     "Patch emails must include a Signed-off-by: line [...] Please use your
>     real name to sign a patch (not an alias or acronym)."
>
>     > Tested-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com
>     <mailto:marcel.apfelbaum@gmail.com>>
>     > [fixed malformed patch, rebase to master]
>     > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com
>     <mailto:marcel.apfelbaum@gmail.com>>
>     > ---
>     >
>     > v2:
>     >  - fixed malformed patch
>     >  - rebased to master
>     >
>     >  ui/console-gl.c | 18 +++++++++++-------
>     >  1 file changed, 11 insertions(+), 7 deletions(-)
>     >
>     > diff --git a/ui/console-gl.c b/ui/console-gl.c
>     > index a56e1cd8eb..c1cb3bd673 100644
>     > --- a/ui/console-gl.c
>     > +++ b/ui/console-gl.c
>     > @@ -92,13 +92,17 @@ void surface_gl_update_texture(QemuGLShader
>     *gls,
>     >
>     >      assert(gls);
>     >
>     > -    glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
>     > -                  surface_stride(surface) /
>     surface_bytes_per_pixel(surface));
>     > -    glTexSubImage2D(GL_TEXTURE_2D, 0,
>     > -                    x, y, w, h,
>     > -                    surface->glformat, surface->gltype,
>     > -                    data + surface_stride(surface) * y
>     > -                    + surface_bytes_per_pixel(surface) * x);
>     > +    if (surface->texture) {
>     > +        glBindTexture(GL_TEXTURE_2D, surface->texture);
>     > +        glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
>     > +                      surface_stride(surface)
>     > +                      / surface_bytes_per_pixel(surface));
>     > +        glTexSubImage2D(GL_TEXTURE_2D, 0,
>     > +                        x, y, w, h,
>     > +                        surface->glformat, surface->gltype,
>     > +                        data + surface_stride(surface) * y
>     > +                        + surface_bytes_per_pixel(surface) * x);
>     > +    }
>     >  }
>     >
>     >  void surface_gl_render_texture(QemuGLShader *gls,
>     >
>
diff mbox series

Patch

diff --git a/ui/console-gl.c b/ui/console-gl.c
index a56e1cd8eb..c1cb3bd673 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -92,13 +92,17 @@  void surface_gl_update_texture(QemuGLShader *gls,
 
     assert(gls);
 
-    glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
-                  surface_stride(surface) / surface_bytes_per_pixel(surface));
-    glTexSubImage2D(GL_TEXTURE_2D, 0,
-                    x, y, w, h,
-                    surface->glformat, surface->gltype,
-                    data + surface_stride(surface) * y
-                    + surface_bytes_per_pixel(surface) * x);
+    if (surface->texture) {
+        glBindTexture(GL_TEXTURE_2D, surface->texture);
+        glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
+                      surface_stride(surface)
+                      / surface_bytes_per_pixel(surface));
+        glTexSubImage2D(GL_TEXTURE_2D, 0,
+                        x, y, w, h,
+                        surface->glformat, surface->gltype,
+                        data + surface_stride(surface) * y
+                        + surface_bytes_per_pixel(surface) * x);
+    }
 }
 
 void surface_gl_render_texture(QemuGLShader *gls,