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)