Source-Changes-D archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys/compat/linux/common
> 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.
We need to do this in a different way. (It's annoying that C doesn't
formally allow the original code, since in principle it is sensible.)
Perhaps we can delete the struct inotify_entry::ie_name member, and
instead of using
struct inotify_event *ie = kmem_zalloc(sizeof(*ie), ...);
do
struct inotify_event *ie = kmem_zalloc(
offsetof(struct inotify_event, ie_event.name[NAME_MAX + 1]),
...);
and similarly for kmem_free.
Home |
Main Index |
Thread Index |
Old Index