kern/60756: compat_linux inotify(2): TOCTOU and locking shenanigans

["campbell+netbsd%mumble.net@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60756
>Category:       kern
>Synopsis:       compat_linux inotify(2): TOCTOU and locking shenanigans
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 20 13:25:01 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, ...
>Organization:
The CompatBSD Inotification, Inc.
>Environment:
>Description:

	inotify_readdir sometimes runs with the vnode lock, and
	sometimes unlocks and relocks the interlock, which strikes
	me as incoherent and likely fundamentally buggy:

    790 	/* XXX: should pass whether to lock or not */
    791 	if (needs_lock)
    792 		vn_lock(vp, LK_SHARED | LK_RETRY);
    793 	else
    794 		/*
    795 		 * XXX We need to temporarily drop v_interlock because
    796 		 * it may be temporarily acquired by biowait().
    797 		 */
    798 		mutex_exit(vp->v_interlock);
    799 	KASSERT(!mutex_owned(vp->v_interlock));
    800 	error = VOP_READDIR(vp, &uio, fp->f_cred, &eofflag, NULL, NULL);
    801 	if (needs_lock)
    802 		VOP_UNLOCK(vp);
    803 	else
    804 		mutex_enter(vp->v_interlock);

	https://nxr.netbsd.org/xref/src/sys/compat/linux/common/linux_inotify.c?r=1.8#761

	It is not clear to me whether in the needs_lock=false case it
	actually holds the vnode lock.  This should be asserted.

	The needs_lock=true case used by get_inotify_dir_entries
	creates a TOCTOU issue: in one pass, it it counts the number of
	entries with inotify_readdir; in a second pass, it copies them
	into an array of that many entries -- but the number of entries
	may have changed between the first pass and the second pass
	(with some large entries being split into multiple small ones),
	possibly leading the inner while loop in the second pass to
	walk off the end of the array.

    842 	mutex_enter(&wp->f_lock);
    843 	wp->f_offset = 0;
    844 	mutex_exit(&wp->f_lock);
    845 	decount = 0;
    846 	for (;;) {
    847 		error = inotify_readdir(wp, &de, &done, needs_lock);
    848 		if (error != 0)
    849 			goto leave;
    850 		if (done == 0)
    851 			break;
    852 
    853 		currdep = &de;
    854 	        while ((char *)currdep < ((char *)&de) + done) {
    855 			decount++;
    856 			currdep = _DIRENT_NEXT(currdep);
    857 		}
    858 	}
    859 
    860 	idep = kmem_zalloc(INOTIFY_DIR_ENTRIES_SIZE(decount), KM_SLEEP);
    861 	idep->ide_count = decount;
    862 
    863 	mutex_enter(&wp->f_lock);
    864 	wp->f_offset = 0;
    865 	mutex_exit(&wp->f_lock);
    866 	for (i = 0; i < decount;) {
    867 		error = inotify_readdir(wp, &de, &done, needs_lock);
    868 		if (error != 0 || done == 0) {
    869 			kmem_free(idep, INOTIFY_DIR_ENTRIES_SIZE(decount));
    870 			idep = NULL;
    871 			goto leave;
    872 		}
    873 
    874 		currdep = &de;
    875 		while ((char *)currdep < ((char *)&de) + done) {
    876 			idep->ide_entries[i].fileno = currdep->d_fileno;
    877 			strcpy(idep->ide_entries[i].name, currdep->d_name);
    878 
    879 			currdep = _DIRENT_NEXT(currdep);
    880 			i++;
    881 		}
    882 	}

	https://nxr.netbsd.org/xref/src/sys/compat/linux/common/linux_inotify.c?r=1.8#842

>How-To-Repeat:

	code inspection

>Fix:

	1. At least bound the inner while loop in the second pass of
	   get_inotify_dir_entries.

	2. Fix the locking shenanigans.

	3. Nix strcpy while here; use strlcpy, strncpy, or copystr
	   instead, depending on the nature of the input and output
	   buffers.