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

[Julien Grall <julien.grall@linaro.org>]

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