On Mon, Jun 5, 2017 at 9:28 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> The old code would just blindly dereference arrays with no protection
> whatsoever. This is a problem because the format_info array is too
> short to contain ISL_FORMAT_UNSUPPORTED or any of the aux formats. This
> can lead to segfaults when the array is dereferenced. This commit
> switches everything over to using a helper for getting isl_format_info
> which does some useful asserting. While we're here, we add the same
> asserts to isl_format_get_layout().
> ---
> src/intel/isl/isl.h | 8 ++++++
> src/intel/isl/isl_format.c | 62
> ++++++++++++++++++++++++++++++++--------------
> 2 files changed, 51 insertions(+), 19 deletions(-)
>
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index 658f67e..5f9baa7 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -389,6 +389,11 @@ enum isl_format {
> ISL_FORMAT_GEN9_CCS_64BPP,
> ISL_FORMAT_GEN9_CCS_128BPP,
>
> + /* This way we can easily tell if it's a valid format. Note:
> + * ISL_FORMAT_UNSUPPORTED is not considered valid.
> + */
> + ISL_FORMAT_MAX_VALID_ENUM,
> +
> /* Hardware doesn't understand this out-of-band value */
> ISL_FORMAT_UNSUPPORTED = UINT16_MAX,
> };
> @@ -1170,6 +1175,9 @@ isl_device_get_sample_counts(struct isl_device *dev);
> static inline const struct isl_format_layout * ATTRIBUTE_CONST
> isl_format_get_layout(enum isl_format fmt)
> {
> + assert(fmt != ISL_FORMAT_UNSUPPORTED);
> + assert(fmt < ISL_FORMAT_MAX_VALID_ENUM);
> +
> return &isl_format_layouts[fmt];
> }
>
> diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c
> index e6d2a43..02e09e9 100644
> --- a/src/intel/isl/isl_format.c
> +++ b/src/intel/isl/isl_format.c
> @@ -21,12 +21,13 @@
> * IN THE SOFTWARE.
> */
>
> +#include <stdlib.h>
> #include <assert.h>
>
> #include "isl.h"
> #include "common/gen_device_info.h"
>
> -struct surface_format_info {
> +struct isl_format_info {
> bool exists;
> uint8_t sampling;
> uint8_t filtering;
> @@ -87,7 +88,7 @@ struct surface_format_info {
> * - VOL4_Part1 section 3.9.11 Render Target Write.
> * - Render Target Surface Types [SKL+]
> */
> -static const struct surface_format_info format_info[] = {
> +static const struct isl_format_info format_infos[] = {
> /* smpl filt shad CK RT AB VB SO color TW TR ccs_e */
> SF( Y, 50, x, x, Y, Y, Y, Y, x, 70, 90, 90, R32G32B32A32_FLOAT)
> SF( Y, x, x, x, Y, x, Y, Y, x, 70, 90, 90, R32G32B32A32_SINT)
> @@ -359,6 +360,21 @@ static const struct surface_format_info format_info[] = {
> #undef x
> #undef Y
>
> +static inline const struct isl_format_info *
> +isl_format_get_info(enum isl_format format)
> +{
> + assert(format != ISL_FORMAT_UNSUPPORTED);
> + assert(format < ISL_FORMAT_MAX_VALID_ENUM);
> +
> + if (format >= ARRAY_SIZE(format_infos))
> + return NULL;
> +
> + if (!format_infos[format].exists)
> + return NULL;
> +
> + return &format_infos[format];
> +}
> +
> static unsigned
> format_gen(const struct gen_device_info *devinfo)
> {
> @@ -369,27 +385,30 @@ bool
> isl_format_supports_rendering(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> - return format_gen(devinfo) >= format_info[format].render_target;
> + return format_gen(devinfo) >= info->render_target;
> }
>
> bool
> isl_format_supports_alpha_blending(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> - return format_gen(devinfo) >= format_info[format].alpha_blend;
> + return format_gen(devinfo) >= info->alpha_blend;
> }
>
> bool
> isl_format_supports_sampling(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> if (devinfo->is_baytrail) {
> @@ -415,14 +434,15 @@ isl_format_supports_sampling(const struct
> gen_device_info *devinfo,
> return true;
> }
>
> - return format_gen(devinfo) >= format_info[format].sampling;
> + return format_gen(devinfo) >= info->sampling;
> }
>
> bool
> isl_format_supports_filtering(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> if (devinfo->is_baytrail) {
> @@ -448,23 +468,24 @@ isl_format_supports_filtering(const struct
> gen_device_info *devinfo,
> return true;
> }
>
> - return format_gen(devinfo) >= format_info[format].filtering;
> + return format_gen(devinfo) >= info->filtering;
> }
>
> bool
> isl_format_supports_vertex_fetch(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> /* For vertex fetch, Bay Trail supports the same set of formats as Haswell
> * but is a superset of Ivy Bridge.
> */
> if (devinfo->is_baytrail)
> - return 75 >= format_info[format].input_vb;
> + return 75 >= info->input_vb;
>
> - return format_gen(devinfo) >= format_info[format].input_vb;
> + return format_gen(devinfo) >= info->input_vb;
> }
>
> /**
> @@ -474,10 +495,11 @@ bool
> isl_format_supports_typed_writes(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> - return format_gen(devinfo) >= format_info[format].typed_write;
> + return format_gen(devinfo) >= info->typed_write;
> }
>
>
> @@ -495,10 +517,11 @@ bool
> isl_format_supports_typed_reads(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> - return format_gen(devinfo) >= format_info[format].typed_read;
> + return format_gen(devinfo) >= info->typed_read;
> }
>
> /**
> @@ -533,10 +556,11 @@ bool
> isl_format_supports_ccs_e(const struct gen_device_info *devinfo,
> enum isl_format format)
> {
> - if (!format_info[format].exists)
> + const struct isl_format_info *info = isl_format_get_info(format);
> + if (info == NULL)
> return false;
>
> - return format_gen(devinfo) >= format_info[format].ccs_e;
> + return format_gen(devinfo) >= info->ccs_e;
> }
>
> bool
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
