Re: CVS commit: src/sys/compat/linux/common

[Taylor R Campbell <campbell+netbsd-source-changes-d%mumble.net@localhost>]

> 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.