On Tue, Apr 05, 2022 at 04:47:18PM +0200, Mauro Matteo Cascella wrote:
> On Tue, Apr 5, 2022 at 1:10 PM Gerd Hoffmann <kra...@redhat.com> wrote:
> >
> > > > +++ b/ui/cursor.c
> > > > @@ -46,6 +46,13 @@ static QEMUCursor *cursor_parse_xpm(const char
> > > > *xpm[])
> > > >
> > > > /* parse pixel data */
> > > > c = cursor_alloc(width, height);
> > > > +
> > > > + if (!c) {
> > > > + fprintf(stderr, "%s: cursor %ux%u alloc error\n",
> > > > + __func__, width, height);
> > > > + return NULL;
> > > > + }
> > > >
> > >
> > > I think you could simply abort() in this function. It is used with static
> > > data (ui/cursor*.xpm)
> >
> > Yes, that should never happen.
> >
> > Missing: vmsvga_cursor_define() calls cursor_alloc() with guest-supplied
> > values too.
>
> I skipped that because the check (cursor.width > 256 || cursor.height
> > 256) is already done in vmsvga_fifo_run before calling
> vmsvga_cursor_define. You want me to add another check in
> vmsvga_cursor_define and return NULL if cursor_alloc fails?
No check required then.
Maybe add an 'assert(c != NULL)' to clarify this should never happen
b/c those call sites never call cursor_alloc() with width/height being
too big (your choice, applies to both vmsvga and ui/cursor.c).
take care,
Gerd
