Bug 253602 - devel/glib20: Local patch to glib20 uses a broken and unstable fdwalk implementation
Summary: devel/glib20: Local patch to glib20 uses a broken and unstable fdwalk impleme...
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-desktop (Team)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-17 22:08 UTC by John Baldwin
Modified: 2021-02-27 04:04 UTC (History)
4 users (show)

See Also:
tcberner: maintainer-feedback+


Attachments
Patch to the port (2.49 KB, patch)
2021-02-17 22:08 UTC, John Baldwin
no flags Details | Diff
build with libutil (305 bytes, patch)
2021-02-19 00:41 UTC, ps
no flags Details | Diff
build with libutil (642 bytes, patch)
2021-02-19 16:52 UTC, ps
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Baldwin freebsd_committer freebsd_triage 2021-02-17 22:08:12 UTC
Created attachment 222539 [details]
Patch to the port

Paul Saab found that devel/glib20 was failing to build in a poudriere jail on one of his machines.  We tracked this down to the alloca() in our local patch to add fdwalk2() failing and overflowing the stack.  The reason is that the fdwalk2() implementation uses the kern.file sysctl which fetches a _global_ table of all open files across all processes, meaning that the amount of required memory scales with the number of processes in the system.  fdwalk2() should instead be using kinfo_getfile() from libutil which fetches only the open files for the current process (which also uses malloc() to avoid stack limit issues).  While here, I also noticed that fdwalk2() failed to propagate the callback's return value to the caller and instead always returns 0 since *ret was never assigned a value.

The attached patch is untested, but should fix both of these issues.  I wasn't quite sure how to convince glib20's port to add libutil as an explicit dependency (e.g. in LDFLAGS), so I'm hopeful a port maintainer can add that missing bit and finish this the rest of the way.
Comment 1 ps 2021-02-19 00:41:48 UTC
Created attachment 222576 [details]
build with libutil
Comment 2 ps 2021-02-19 16:52:14 UTC
Created attachment 222645 [details]
build with libutil
Comment 3 John Baldwin freebsd_committer freebsd_triage 2021-02-23 18:47:45 UTC
Full patch now up for review at https://reviews.freebsd.org/D28904.
Comment 4 commit-hook freebsd_committer 2021-02-26 19:31:58 UTC
A commit references this bug:

Author: jhb
Date: Fri Feb 26 19:31:05 UTC 2021
New revision: 566632
URL: https://svnweb.freebsd.org/changeset/ports/566632

Log:
  Use kinfo_getfile() to implement fdwalk().

  Previously, the kern.file sysctl (which queries the global file table)
  was queried and the results saved in an on-stack buffer.  With a
  sufficiently active system the sysctl's output could overflow the
  stack's available space.  Instead, switch to kinfo_getfile() from
  libutil.  This uses a sysctl which queries only the open files for the
  current process, and it uses heap space instead of the stack to store
  the sysctl output.

  PR:		253602
  Submitted by:	ps (build glue patches)
  Reported by:	ps
  Reviewed by:	bapt
  MFH:		2021Q1
  Differential Revision:	https://reviews.freebsd.org/D28904

Changes:
  head/devel/glib20/Makefile
  head/devel/glib20/files/patch-glib_gspawn.c
  head/devel/glib20/files/patch-glib_meson.build
  head/devel/glib20/files/patch-meson.build
Comment 5 fullermd 2021-02-26 23:28:47 UTC
For a data point, I also had glib failing in poudriere on one machine, and with the committed changes it now works.
Comment 6 Tobias C. Berner freebsd_committer 2021-02-27 04:04:09 UTC
Thanks for fixing this.

mfg Tobias