ramdac: Check sPriv != NULL in xf86CheckHWCursor()

Submitted by Alex Goins on Oct. 24, 2016, 10:25 p.m.

Details

Message ID 1477347953-9687-1-git-send-email-agoins@nvidia.com
State Accepted
Headers show
Series "ramdac: Check sPriv != NULL in xf86CheckHWCursor()" ( rev: 1 ) in X.org (DEPRECATED - USE GITLAB)

Not browsing as part of any series.

Commit Message

Alex Goins Oct. 24, 2016, 10:25 p.m. 
xf86CheckHWCursor() would dereference sPriv without NULL checking it. If Option
"SWCursor" is specified, sPriv == NULL. In this case we should assume that HW
cursors are not supported.

Signed-off-by: Alex Goins <agoins@nvidia.com>
Reviewed-by: Andy Ritger <aritger@nvidia.com>
---
 hw/xfree86/ramdac/xf86HWCurs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox 

diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index da2b181..5e99526 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -148,7 +148,8 @@  xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr
             continue;
 
         sPriv = dixLookupPrivate(&pSlave->devPrivates, xf86CursorScreenKey);
-        if (!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
+        if (!sPriv ||
+            !xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
             return FALSE;
     }
     return TRUE;

Comments

Walter Harms Oct. 25, 2016, 8:10 a.m. 
Am 25.10.2016 00:25, schrieb Alex Goins:
> xf86CheckHWCursor() would dereference sPriv without NULL checking it. If Option
> "SWCursor" is specified, sPriv == NULL. In this case we should assume that HW
> cursors are not supported.
> 
> Signed-off-by: Alex Goins <agoins@nvidia.com>
> Reviewed-by: Andy Ritger <aritger@nvidia.com>
> ---
>  hw/xfree86/ramdac/xf86HWCurs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
> index da2b181..5e99526 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -148,7 +148,8 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr
>              continue;
>  
>          sPriv = dixLookupPrivate(&pSlave->devPrivates, xf86CursorScreenKey);
> -        if (!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
> +        if (!sPriv ||
> +            !xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
>              return FALSE;
>      }
>      return TRUE;


I would go for 2 lines that will make things more clear also adding a comment would help
future debugging operations.

1. if (!sPriv) return FALSE;  /* maybe option "SWCursor" */

2. if (!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))  /* HWCursor not supported */


just my 2 cents,

re,
 wh
Hans de Goede Oct. 25, 2016, 8:11 a.m. 
Hi,

On 25-10-16 00:25, Alex Goins wrote:
> xf86CheckHWCursor() would dereference sPriv without NULL checking it. If Option
> "SWCursor" is specified, sPriv == NULL. In this case we should assume that HW
> cursors are not supported.
>
> Signed-off-by: Alex Goins <agoins@nvidia.com>
> Reviewed-by: Andy Ritger <aritger@nvidia.com>

Patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  hw/xfree86/ramdac/xf86HWCurs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
> index da2b181..5e99526 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -148,7 +148,8 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr
>              continue;
>
>          sPriv = dixLookupPrivate(&pSlave->devPrivates, xf86CursorScreenKey);
> -        if (!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
> +        if (!sPriv ||
> +            !xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
>              return FALSE;
>      }
>      return TRUE;
>