Re: [Xen-devel] [PATCH 1/4] Expand Xen IOMEMCON to 64 bits

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Daniel,

On 12/03/15 17:12, Daniel De Graaf wrote:
> diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> index d03dc20..d98a5eb 100644
> --- a/libsepol/src/write.c
> +++ b/libsepol/src/write.c
> @@ -1252,13 +1252,24 @@ static int ocontext_write_xen(struct 
> policydb_compat_info *info, policydb_t *p,
>                                       return POLICYDB_ERROR;
>                               break;
>                       case OCON_XEN_IOMEM:
> -                             buf[0] = c->u.iomem.low_iomem;
> -                             buf[1] = c->u.iomem.high_iomem;
> -                             for (j = 0; j < 2; j++)
> -                                     buf[j] = cpu_to_le32(buf[j]);
> -                             items = put_entry(buf, sizeof(uint32_t), 2, fp);
> -                             if (items != 2)
> -                                     return POLICYDB_ERROR;
> +                             if (p->policyvers >= 
> POLICYDB_XEN_VERSION_AARCH) {
> +                                     uint64_t b64[2];
> +                                     b64[0] = c->u.iomem.low_iomem;
> +                                     b64[1] = c->u.iomem.high_iomem;
> +                                     for (j = 0; j < 2; j++)
> +                                             b64[j] = cpu_to_le64(b64[j]);
> +                                     items = put_entry(b64, 
> sizeof(uint64_t), 2, fp);
> +                                     if (items != 2)
> +                                             return POLICYDB_ERROR;
> +                             } else {
> +                                     buf[0] = c->u.iomem.low_iomem;
> +                                     buf[1] = c->u.iomem.high_iomem;
> +                                     for (j = 0; j < 2; j++)
> +                                             buf[j] = cpu_to_le32(buf[j]);
> +                                     items = put_entry(buf, 
> sizeof(uint32_t), 2, fp);
> +                                     if (items != 2)
> +                                             return POLICYDB_ERROR;

If low_iomem/high_iomem doesn't fit in an uint32_t it will be truncated
and therefore we may, by mistake, give access to wrong MMIO region.

Shouldn't we at least add a warning, if not throwing an error?

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel