Sure, that makes sense. Reviewed-by: Jamey Sharp <ja...@minilop.net>
On 11/23/11, Alan Coopersmith <alan.coopersm...@oracle.com> wrote: > Commit f9e3a2955d2ca7 removing the MAXSCREEN limit left the screen > number too unlimited, and allowed any positive int for a screen number: > > Xvfb :1 -screen 2147483647 1024x1024x8 > > Fatal server error: > Not enough memory for screen 2147483647 > > Found by Parfait 0.3.7: > Error: Integer overflow (CWE 190) > Integer parameter of memory allocation function realloc() may overflow > due to multiplication with constant value 1112 > at line 293 of hw/vfb/InitOutput.c in function 'ddxProcessArgument'. > > Since the X11 connection setup only has a CARD8 for number of SCREENS, > limit to 255 screens, which is also low enough to avoid overflow on the > sizeof(*vfbScreens) * (screenNum + 1) calculation for realloc. > > Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> > --- > hw/vfb/InitOutput.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c > index 3e5d051..9a9905d 100644 > --- a/hw/vfb/InitOutput.c > +++ b/hw/vfb/InitOutput.c > @@ -280,7 +280,9 @@ ddxProcessArgument(int argc, char *argv[], int i) > int screenNum; > CHECK_FOR_REQUIRED_ARGUMENTS(2); > screenNum = atoi(argv[i+1]); > - if (screenNum < 0) > + /* The protocol only has a CARD8 for number of screens in the > + connection setup block, so don't allow more than that. */ > + if ((screenNum < 0) || (screenNum >= 255)) > { > ErrorF("Invalid screen number %d\n", screenNum); > UseMsg(); > -- > 1.7.3.2 > > _______________________________________________ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel