| Message ID | 1345172107-27092-1-git-send-email-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
On 17.08.2012 06:55, Marek Vasut wrote: > Disallow negative value boundaries of the redraw rectangle. > This fixes a segfault when using -vga vmware. > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > hw/vmware_vga.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > NOTE: I tested this by emulating some recent version of ubuntu. The rect->x > value was set to -65 for some reason at one point, which caused the > kvm to crash. Trimming the rectangle fixed the issue. > > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > index f5e4f44..62e5887 100644 > --- a/hw/vmware_vga.c > +++ b/hw/vmware_vga.c > @@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s, > { > struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last ++]; > s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; > - rect->x = x; > - rect->y = y; > + rect->x = (x < 0) ? 0 : x; > + rect->y = (y < 0) ? 0 : y; > rect->w = w; > rect->h = h; > } Is it the same as https://bugs.launchpad.net/bugs/918791 ? At least it appears to be the same theme... But there, the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) also updates width/height. My comment: https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21 "So indeed, some (upstream) verification is needed here -- where these negative values are coming from, whenever it is EVER okay to have them, what to do with these, and where to check (I guess the check should be done somewhere in the upper layer)." Especially the last part about the layer. Thanks, /mjt
Dear Michael Tokarev, > On 17.08.2012 06:55, Marek Vasut wrote: > > Disallow negative value boundaries of the redraw rectangle. > > This fixes a segfault when using -vga vmware. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > --- > > > > hw/vmware_vga.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > NOTE: I tested this by emulating some recent version of ubuntu. The > > rect->x > > > > value was set to -65 for some reason at one point, which caused the > > kvm to crash. Trimming the rectangle fixed the issue. > > > > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > > index f5e4f44..62e5887 100644 > > --- a/hw/vmware_vga.c > > +++ b/hw/vmware_vga.c > > @@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct > > vmsvga_state_s *s, > > > > { > > > > struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last > > ++]; s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; > > > > - rect->x = x; > > - rect->y = y; > > + rect->x = (x < 0) ? 0 : x; > > + rect->y = (y < 0) ? 0 : y; > > > > rect->w = w; > > rect->h = h; > > > > } > > Is it the same as https://bugs.launchpad.net/bugs/918791 ? > At least it appears to be the same theme... But there, > the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) > also updates width/height. My comment: > https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21 Looks to be the same ... though my patch tries to squash the issue as early as possible. You're right that x and y might overflow to the other side too. Also, you're right about w and h. Shall I send updated patch? > "So indeed, some (upstream) verification is needed here -- where these > negative values are coming from, whenever it is EVER okay to have them, > what to do with these, and where to check (I guess the check should be > done somewhere in the upper layer)." > > Especially the last part about the layer. Where's the upper layer though, isn't that what's pouring out of the virtual machine itself? > Thanks, Thank you for guidance ! > /mjt Best regards, Marek Vasut
> Dear Michael Tokarev, Bump? Did this lead anywhere? Do you need updated patch? > > On 17.08.2012 06:55, Marek Vasut wrote: > > > Disallow negative value boundaries of the redraw rectangle. > > > This fixes a segfault when using -vga vmware. > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > --- > > > > > > hw/vmware_vga.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > NOTE: I tested this by emulating some recent version of ubuntu. The > > > rect->x > > > > > > value was set to -65 for some reason at one point, which caused > > > the kvm to crash. Trimming the rectangle fixed the issue. > > > > > > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > > > index f5e4f44..62e5887 100644 > > > --- a/hw/vmware_vga.c > > > +++ b/hw/vmware_vga.c > > > @@ -337,8 +337,8 @@ static inline void > > > vmsvga_update_rect_delayed(struct vmsvga_state_s *s, > > > > > > { > > > > > > struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last > > > ++]; s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; > > > > > > - rect->x = x; > > > - rect->y = y; > > > + rect->x = (x < 0) ? 0 : x; > > > + rect->y = (y < 0) ? 0 : y; > > > > > > rect->w = w; > > > rect->h = h; > > > > > > } > > > > Is it the same as https://bugs.launchpad.net/bugs/918791 ? > > At least it appears to be the same theme... But there, > > the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) > > also updates width/height. My comment: > > https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/2 > > 1 > > Looks to be the same ... though my patch tries to squash the issue as early > as possible. > > You're right that x and y might overflow to the other side too. Also, > you're right about w and h. > > Shall I send updated patch? > > > "So indeed, some (upstream) verification is needed here -- where these > > negative values are coming from, whenever it is EVER okay to have them, > > what to do with these, and where to check (I guess the check should be > > done somewhere in the upper layer)." > > > > Especially the last part about the layer. > > Where's the upper layer though, isn't that what's pouring out of the > virtual machine itself? > > > Thanks, > > Thank you for guidance ! > > > /mjt > > Best regards, > Marek Vasut Best regards, Marek Vasut
On 17.09.2012 00:36, Marek Vasut wrote: >> Dear Michael Tokarev, > > Bump? Did this lead anywhere? Do you need updated patch? This didn't lead to anywhere. I just pointed out that we have similar issue and somewhat similar change, and it weren't sufficient. I don't know this code, and have no idea who does, either. Thanks, /mjt >>> On 17.08.2012 06:55, Marek Vasut wrote: >>>> Disallow negative value boundaries of the redraw rectangle. >>>> This fixes a segfault when using -vga vmware. >>>> >>>> Signed-off-by: Marek Vasut <marex@denx.de> >>>> --- >>>> >>>> hw/vmware_vga.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> NOTE: I tested this by emulating some recent version of ubuntu. The >>>> rect->x >>>> >>>> value was set to -65 for some reason at one point, which caused >>>> the kvm to crash. Trimming the rectangle fixed the issue. >>>> >>>> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c >>>> index f5e4f44..62e5887 100644 >>>> --- a/hw/vmware_vga.c >>>> +++ b/hw/vmware_vga.c >>>> @@ -337,8 +337,8 @@ static inline void >>>> vmsvga_update_rect_delayed(struct vmsvga_state_s *s, >>>> >>>> { >>>> >>>> struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last >>>> ++]; s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; >>>> >>>> - rect->x = x; >>>> - rect->y = y; >>>> + rect->x = (x < 0) ? 0 : x; >>>> + rect->y = (y < 0) ? 0 : y; >>>> >>>> rect->w = w; >>>> rect->h = h; >>>> >>>> } >>> >>> Is it the same as https://bugs.launchpad.net/bugs/918791 ? >>> At least it appears to be the same theme... But there, >>> the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) >>> also updates width/height. My comment: >>> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/2 >>> 1 >> >> Looks to be the same ... though my patch tries to squash the issue as early >> as possible. >> >> You're right that x and y might overflow to the other side too. Also, >> you're right about w and h. >> >> Shall I send updated patch? >> >>> "So indeed, some (upstream) verification is needed here -- where these >>> negative values are coming from, whenever it is EVER okay to have them, >>> what to do with these, and where to check (I guess the check should be >>> done somewhere in the upper layer)." >>> >>> Especially the last part about the layer. >> >> Where's the upper layer though, isn't that what's pouring out of the >> virtual machine itself? >> >>> Thanks, >> >> Thank you for guidance ! >> >>> /mjt >> >> Best regards, >> Marek Vasut > > Best regards, > Marek Vasut
Dear Michael Tokarev, > On 17.09.2012 00:36, Marek Vasut wrote: > >> Dear Michael Tokarev, > > > > Bump? Did this lead anywhere? Do you need updated patch? > > This didn't lead to anywhere. I just pointed out that we > have similar issue and somewhat similar change, and it > weren't sufficient. I don't know this code, and have no > idea who does, either. So what do we do? I'll try to investigate a bit more, all right? > Thanks, > > /mjt > > >>> On 17.08.2012 06:55, Marek Vasut wrote: > >>>> Disallow negative value boundaries of the redraw rectangle. > >>>> This fixes a segfault when using -vga vmware. > >>>> > >>>> Signed-off-by: Marek Vasut <marex@denx.de> > >>>> --- > >>>> > >>>> hw/vmware_vga.c | 4 ++-- > >>>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>>> > >>>> NOTE: I tested this by emulating some recent version of ubuntu. The > >>>> rect->x > >>>> > >>>> value was set to -65 for some reason at one point, which caused > >>>> the kvm to crash. Trimming the rectangle fixed the issue. > >>>> > >>>> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c > >>>> index f5e4f44..62e5887 100644 > >>>> --- a/hw/vmware_vga.c > >>>> +++ b/hw/vmware_vga.c > >>>> @@ -337,8 +337,8 @@ static inline void > >>>> vmsvga_update_rect_delayed(struct vmsvga_state_s *s, > >>>> > >>>> { > >>>> > >>>> struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last > >>>> ++]; s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; > >>>> > >>>> - rect->x = x; > >>>> - rect->y = y; > >>>> + rect->x = (x < 0) ? 0 : x; > >>>> + rect->y = (y < 0) ? 0 : y; > >>>> > >>>> rect->w = w; > >>>> rect->h = h; > >>>> > >>>> } > >>> > >>> Is it the same as https://bugs.launchpad.net/bugs/918791 ? > >>> At least it appears to be the same theme... But there, > >>> the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) > >>> also updates width/height. My comment: > >>> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments > >>> /2 1 > >> > >> Looks to be the same ... though my patch tries to squash the issue as > >> early as possible. > >> > >> You're right that x and y might overflow to the other side too. Also, > >> you're right about w and h. > >> > >> Shall I send updated patch? > >> > >>> "So indeed, some (upstream) verification is needed here -- where these > >>> negative values are coming from, whenever it is EVER okay to have them, > >>> what to do with these, and where to check (I guess the check should be > >>> done somewhere in the upper layer)." > >>> > >>> Especially the last part about the layer. > >> > >> Where's the upper layer though, isn't that what's pouring out of the > >> virtual machine itself? > >> > >>> Thanks, > >> > >> Thank you for guidance ! > >> > >>> /mjt > >> > >> Best regards, > >> Marek Vasut > > > > Best regards, > > Marek Vasut
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index f5e4f44..62e5887 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s, { struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last ++]; s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1; - rect->x = x; - rect->y = y; + rect->x = (x < 0) ? 0 : x; + rect->y = (y < 0) ? 0 : y; rect->w = w; rect->h = h; }
Disallow negative value boundaries of the redraw rectangle. This fixes a segfault when using -vga vmware. Signed-off-by: Marek Vasut <marex@denx.de> --- hw/vmware_vga.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) NOTE: I tested this by emulating some recent version of ubuntu. The rect->x value was set to -65 for some reason at one point, which caused the kvm to crash. Trimming the rectangle fixed the issue.