Hi Stefano,
On 23/03/2022 00:08, Stefano Stabellini wrote:
On Sat, 29 Jan 2022, Julien Grall wrote:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6931c022a2..9144d6c0b6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2963,6 +2963,7 @@ static int __init construct_domU(struct domain *d,
const struct dt_device_node *node)
{
struct kernel_info kinfo = {};
+ const char *dom0less_enhanced;
int rc;
u64 mem;
@@ -2978,6 +2979,12 @@ static int __init construct_domU(struct domain *d,
kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
+ rc = dt_property_read_string(node, "xen,enhanced",
&dom0less_enhanced);
+ if ( rc == -EILSEQ ||
I think the use an -EILSEQ wants an explanation. In a previous version, you
wrote that the value would be returned when:
fdt set /chosen/domU0 xen,enhanced
But it is not clear why. Can you print pp->value, pp->length, strnlen(..) when
this happens?
I added in dt_property_read_string:
printk("DEBUG %s %d value=%s value[0]=%d length=%u
len=%lu\n",__func__,__LINE__,(char*)pp->value, *((char*)pp->value),pp->length,
strlen(pp->value));
This is the output:
(XEN) DEBUG dt_property_read_string 205 value= value[0]=0 length=0 len=0
Thanks posting the log!
For convenience, I am copying the comment on top of
dt_property_read_string() prototype:
* Search for a property in a device tree node and retrieve a null
* terminated string value (pointer to data, not a copy). Returns 0 on
* success, -EINVAL if the property does not exist, -ENODATA if property
* doest not have value, and -EILSEQ if the string is not
* null-terminated with the length of the property data.
Per your log, the length is NULL so I would have assumed -ENODATA would
be returned. Looking at the implementation:
const struct dt_property *pp = dt_find_property(np, propname, NULL);
if ( !pp )
return -EINVAL;
if ( !pp->value )
return -ENODATA;
if ( strnlen(pp->value, pp->length) >= pp->length )
return -EILSEQ;
We consider that the property when pp->value is NULL. However, AFAICT,
we never set pp->value to NULL (see unflatten_dt_node()).
So I think there is a bug in the implementation. I would keep the check
!pp->value (for hardening purpose) and also return -ENODATA when
!pp->length.
Most of our device-tree code is from Linux. Looking at v5.17, the bug
seems to be present there too. This would want to be fixed there too.
Cheers,
--
Julien Grall
