On 08/01/2020 15:20, Alexey Kardashevskiy wrote:
>
>
> On 07/01/2020 16:54, David Gibson wrote:
>> On Tue, Jan 07, 2020 at 03:44:35PM +1100, Alexey Kardashevskiy wrote:
>>>
>>>
>>> On 06/01/2020 15:19, David Gibson wrote:
>>>>> +
>>>>> +static uint32_t client_package_to_path(const void *fdt, uint32_t phandle,
>>>>> + uint32_t buf, uint32_t len)
>>>>> +{
>>>>> + char tmp[256];
>>>>
>>>> Fixed sized buffers are icky. You could either dynamically allocate
>>>> this based on the size the client gives, or you could use
>>>> memory_region_get_ram_ptr() to read the data from the tree directly
>>>> into guest memory.
>>>
>>> @len comes from the guest, I am really not comfortable with allocating
>>> whatever (broken) guest requested. And if I limit @len by 1024 or
>>> similar, then a fixed size buffer will do too, no?
>>
>> I see your point. Does this call have a way to report failure? In
>> that case you could outright fail the call if it requests too long a
>> length.
>
> It returns length which can be 0 to signal an error.
>
> but with this particular method the bigger problem is that I cannot know
> in advance the actual path length from fdt_get_path(). I could double
> the size until fdt_get_path() succeeded, just seems overkill here.
>
> Property names seem to be limited by 32:
>>> len("ibm,query-interrupt-source-number")
33
Awesome. Oh well :(
>
> OF1275:
> ===
> nextprop
> IN:phandle, [string] previous, [address] buf
> OUT: flag
>
> Copies the name of the property following previous in the property list
> of the device node identified by phandle into buf, as a null-terminated
> string. Buf is the address of a 32-byte region of memory. If previous is
> zero or a pointer to a null string, copies the name of the device node’s
> first property.
> ===
>
>
>>> btw how exactly can I use memory_region_get_ram_ptr()?
>>> get_system_memory() returns a root MR which is not RAM, RAM is a
>>> "spapr.ram" sub-MR.
>>
>> Right, but you know that RAM is always at offset 0 within that root
>> MR.
>
> Well, it could potentially be more than just one level down in the MR
> tree, for example we could add NUMA MRs and place actual RAM MR under these.
>
>
>> That said, it doesn't look like it's that easy to bounds check
>> that pointer, so maybe that's not a good idea after all.
>
> ok.
>
>
--
Alexey
