Re: [Xen-devel] [PATCH v6 04/12] xen: add basic hypervisor filesystem support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.03.2020 07:06, Jürgen Groß wrote:
> On 04.03.20 16:21, Jan Beulich wrote:
>> On 04.03.2020 16:14, Jürgen Groß wrote:
>>> On 04.03.20 16:07, Jan Beulich wrote:
>>>> On 04.03.2020 15:39, Jürgen Groß wrote:
>>>>> On 04.03.20 14:03, Jan Beulich wrote:
>>>>>> On 04.03.2020 13:00, Jürgen Groß wrote:
>>>>>>> It is perfectly valid to write a shorter string into a character
>>>>>>> array. I could drop the blob here, but in the end I think allowing
>>>>>>> for a blob to change the size should be fine.
>>>>>>
>>>>>> But shouldn't this then also adjust the recorded size?
>>>>>
>>>>> No, this is the max size of the buffer (you can have a look at patch 9
>>>>> where the size is set to the provided space for custom and string
>>>>> parameters).
>>>>
>>>> If I'm not mistaken it is hypfs_read_leaf() which processes read
>>>> requests for strings. Yet that copies entry->size bytes, not the
>>>> potentially smaller strlen()-bounded payload. Things would be
>>>
>>> There is no risk of leaking problematic data here.
>>
>> I didn't think of leaks, but rather of consumers looking at the
>> size and strlen() and getting confused about the mismatch.
> 
> I think telling the maximum possible write length is mandatory.
> 
> So either I can add a comment to the header saying that for strings
> and blobs the length is the maximum value and the content is to be
> self-descriptive regarding its true length (which is the case for
> strings due to the terminating 0 byte), or I need two size fields:
> one for the actual size and one for the maximum allowed size for
> writes (this could then replace the writable flag with "0" for "not
> writable").

Personally I'd prefer the latter, but I could also live with a
comment. The self-descriptive part may, for blobs or gzip-ed
data, be problematic though.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel