When called with ndp->ni_startdir==NULL && ndp->ni_dirfd!=AT_FDCWD, namei() -> namei_setup() fills ndp->ni_filecaps via fget_cap() with possibly (but seems not on most systems) dynamically allocated data. filecaps_free() is needed somewhere after that, but there is no. ndp->ni_startdir==NULL && ndp->ni_dirfd!=AT_FDCWD is usually a result of NDINIT_AT() or NDINIT_ATRIGHTS() macros with externally specified fd. Found places (I think there is mre): uipc_usrreq.c uipc_bindat(), unp_connectat() vfs_cache.c kern___realpathat() vfs_syscalls() a lot of kern_*at(); kern_openat() is an exception: it has proper filecaps_free() at least on success branch (but not on errors)
Good find. I'll attach a program which demonstrates the leak. I'm not sure how to fix it. Certainly it's not practical to patch all namei() users. But most of them don't inspect caps in the first place, so perhaps they should be copied on demand...?
Created attachment 232680 [details] reproducer Reproducer using renameat(), though there are lots of ways to trigger it.
(In reply to Mark Johnston from comment #2) This is rather slow, but can be tracked easily: # vmstat -m | grep filecaps filecaps 34596888 540577K 34596961 16,32,64
I agree it should be on demand, but I have further notes. 1. fd-related code in namei_setup needs to be factored away to kern_descrip.c. 2. if the caller does not care to look at caps, we can make a copy without allocating anything (and subsequently without having to free anything). as shown in the reproducer, the only part which requires malloc is ioctl handling, but ioctls are not inspected in the lookup itself. 3. instead of direct filecaps_free perhaps NDFREE_CAPS or similar should be provided I'll sleep on it and probably hack it up over the weekend. It will be mfcable.
This is the almost minimal patch for the immediate fix: https://reviews.freebsd.org/D34667
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=0c805718cbd3709e3ffc1a0d41612168c8242360 commit 0c805718cbd3709e3ffc1a0d41612168c8242360 Author: Mateusz Guzik <mjg@FreeBSD.org> AuthorDate: 2022-03-24 20:51:03 +0000 Commit: Mateusz Guzik <mjg@FreeBSD.org> CommitDate: 2022-04-02 12:09:07 +0000 vfs: fix memory leak on lookup with fds with ioctl caps Reviewed by: markj PR: 262515 Noted by: firk@cantconnect.ru Differential Revision: https://reviews.freebsd.org/D34667 sys/kern/kern_descrip.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++- sys/kern/vfs_cache.c | 2 +- sys/kern/vfs_lookup.c | 50 ++------------------------------- sys/kern/vfs_syscalls.c | 8 +++--- sys/sys/file.h | 1 + sys/sys/namei.h | 7 ++++- 6 files changed, 88 insertions(+), 55 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=838d8e6fb60e12e610701ae10be717309f3ea935 commit 838d8e6fb60e12e610701ae10be717309f3ea935 Author: Mateusz Guzik <mjg@FreeBSD.org> AuthorDate: 2022-03-24 20:51:03 +0000 Commit: Mateusz Guzik <mjg@FreeBSD.org> CommitDate: 2022-04-04 19:16:18 +0000 vfs: fix memory leak on lookup with fds with ioctl caps Reviewed by: markj PR: 262515 Noted by: firk@cantconnect.ru Differential Revision: https://reviews.freebsd.org/D34667 (cherry picked from commit 0c805718cbd3709e3ffc1a0d41612168c8242360) sys/kern/kern_descrip.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++- sys/kern/vfs_cache.c | 3 +- sys/kern/vfs_lookup.c | 50 ++------------------------------- sys/kern/vfs_syscalls.c | 8 +++--- sys/sys/file.h | 1 + sys/sys/namei.h | 7 ++++- 6 files changed, 89 insertions(+), 55 deletions(-)
A commit in branch releng/13.1 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=fffe016c8155805d1bd957a73a612f2c5e6ceac0 commit fffe016c8155805d1bd957a73a612f2c5e6ceac0 Author: Mateusz Guzik <mjg@FreeBSD.org> AuthorDate: 2022-03-24 20:51:03 +0000 Commit: Mateusz Guzik <mjg@FreeBSD.org> CommitDate: 2022-04-06 23:25:50 +0000 vfs: fix memory leak on lookup with fds with ioctl caps Reviewed by: markj PR: 262515 Noted by: firk@cantconnect.ru Differential Revision: https://reviews.freebsd.org/D34667 Approved by: re (gjb) (cherry picked from commit 0c805718cbd3709e3ffc1a0d41612168c8242360) (cherry picked from commit 838d8e6fb60e12e610701ae10be717309f3ea935) sys/kern/kern_descrip.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++- sys/kern/vfs_cache.c | 3 +- sys/kern/vfs_lookup.c | 50 ++------------------------------- sys/kern/vfs_syscalls.c | 8 +++--- sys/sys/file.h | 1 + sys/sys/namei.h | 7 ++++- 6 files changed, 89 insertions(+), 55 deletions(-)