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
