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 |
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, >
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, > > >
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, > > >
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 --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,