Bruce, See inline comments below.

I'll ACK this patch when we've made sure the HEADER3 and HEADER4 are not 
violating the security policy.

[email protected] wrote:
> Hello All:
>     This is patch 4 of the 5 patches. It's based on 
> 2.6.32-rc3+patch1+patch2+patch3. It fixes the bug of via_verify.c function.
>
> 3 files has been modified for this patch.
>
> Sign-off by Bruce C. Chang <[email protected]>
> diff -Nur linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_3d_reg.h 
> linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_3d_reg.h
> --- linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_3d_reg.h     2009-10-05 
> 08:12:30.000000000 +0800
> +++ linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_3d_reg.h     2009-10-08 
> 10:19:12.000000000 +0800
> @@ -1647,4 +1647,7 @@
>  #define VIA_VIDEO_HEADER6       0xFE050000
>  #define VIA_VIDEO_HEADER7       0xFE060000
>  #define VIA_VIDEOMASK           0xFFFF0000
> +#define HALCYON_HEADER3         0xF4000000
> +#define HALCYON_HEADER4         0xF6000000
> +#define HALCYON_HEADER_MASK     0xFE000000
>   

You make the HEADER3 and HEADER4 commands bypass the verifier. Could you 
please briefly describe what these commands do, or point us to some 
usable documentation?

>  #endif
> diff -Nur linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_verifier.c 
> linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_verifier.c
> --- linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_verifier.c   2009-10-05 
> 08:12:30.000000000 +0800
> +++ linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_verifier.c   2009-10-08 
> 10:19:12.000000000 +0800
> @@ -41,6 +41,8 @@
>       state_header1,
>       state_vheader5,
>       state_vheader6,
> +     state_header3,
> +     state_header4,
>       state_error
>  } verifier_state_t;
>  
> @@ -227,7 +229,10 @@
>       {0xf2, check_for_header2_err},
>       {0xf0, check_for_header1_err},
>       {0xcc, check_for_dummy},
> -     {0x00, check_number_texunits}
> +     {0x00, check_number_texunits},
> +     {0x01, no_check},
> +     {0x02, no_check},
> +     {0x03, no_check}
>  };
>  
>   

The above is for YUV -> RGB conversion, right?

> @@ -791,10 +797,11 @@
>               return 1;
>       }
>       while (dwords--) {
> -             if (*buf++) {
> +             if (*buf && !is_dummy_cmd(*buf)) {
>                       DRM_ERROR("Illegal video command tail.\n");
>                       return 1;
>               }
> +             buf++;
>       }
>       *buffer = buf;
>       return 0;
>   

Why can't the user-space applications just add 0 to pad the video 
command tails?
Even if the hardware accepts dummy commands, the test above adds time to 
the verification process.

/Thomas



------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to