[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT RFC PATCH] plat/common:Add helper to simplify accessing fdt for arm
Hi, On 11/15/18 10:11 AM, Jianyong Wu (Arm Technology China) wrote: -----Original Message----- From: Julien Grall <julien.grall@xxxxxxx> Sent: Thursday, November 15, 2018 5:52 PM To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@xxxxxxx>; minios- devel@xxxxxxxxxxxxxxxxxxxx; simon.kuenzer@xxxxxxxxx Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>; Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx> Subject: Re: [Minios-devel] [UNIKRAFT RFC PATCH] plat/common:Add helper to simplify accessing fdt for arm On 11/15/18 6:09 AM, Jianyong Wu (Arm Technology China) wrote:Hi,Hi,+ + UK_ASSERT(device != -1); + naddr = fdt_address_cells(dtb, device); + UK_ASSERT(naddr < FDT_MAX_NCELLS); + + *nsize = fdt_size_cells(dtb, device); + UK_ASSERT(*nsize < FDT_MAX_NCELLS); + + *regs = fdt_getprop(dtb, device, "reg", &prop_len); + prop_min_len = (int)sizeof(fdt32_t) * (naddr + *nsize); + UK_ASSERT(*regs != NULL && prop_len >= prop_min_len);This assert is not very useful for "regs" property describing more than 1 regions. I think it would make sense to move the check in the uk_dtb_read_term to check if the region requested by the caller iscorrect.Ok, I will check reg in uk_dtb_read_term.But, how to check reg in uk_dtb_read_term? I have an idea that check *(reg + index*(naddr+nsize) *4) Does that make sence?You want to make sure you are not going to read past the size of the property. So ((index + 1) * (naddr + nsize) * 4) < size should be fine.The "size" in this function is not the size of all reg, it is just the length of one term like distributor in gic. We not get the all regs cell size in this function. Oh sorry, I misread the code sorry.We definitely want some safety here, so it is probably a call to rework the interface. I remembered you dismissed in a early revision, but I think it is worthwhile to reconsider the following interface: uk_dtb_read_reg(int node, unsigned int index, &size);You can then add all the safety and also avoid to have to add more parameters to this function. What do you think? Cheers, -- Julien Grall _______________________________________________ Minios-devel mailing list Minios-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/minios-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |