On 2023-08-25 13:13, Taylor R Campbell wrote:
>> Module Name: src
>> Committed By: christos
>> Date: Wed Aug 23 19:17:59 UTC 2023
>>
>> Modified Files:
>> src/sys/compat/linux/common: linux_inotify.c
>>
>> Log Message:
>> put variable length structure at the end, so that clang does not complain.
>>
>> struct inotify_entry {
>> TAILQ_ENTRY(inotify_entry) ie_entries;
>> + char ie_name[NAME_MAX + 1];
>> struct linux_inotify_event ie_event;
>> - char ie_name[NAME_MAX+1];
>> };
>
> This can't be right, and it's a little unsettling that the problem
> isn't caught by any automatic tests.
>
> As I understand it, the `ie_name' member is supposed to provide
> storage for the `ie_event.name' array.
>
> So this looks like this change is going to lead either to a buffer
> overrun -- reading someone else's heap memory, or scribbling all over
> someone else's heap memory -- or to uninitialized or zero-filled
> memory being published to userland where there should be a pathname.
I don't think so. Notice that in inotify_read() two calls to uiomove()
are made, one on ie_event and one on ie_name... and in general this code
never accesses ie_event.name directly.
The reason I separated the fields in the first place was to make
allocation/deallocation simpler, not to use ie_name as storage for
ie_event.name.
Theo(dore)
