Date: Fri, 28 Jul 2023 22:47:51 +0000 From: bugzilla-noreply@freebsd.org To: bugs@FreeBSD.org Subject: [Bug 258559] tcsh can crash in morecore() due to 32-bit arithmetic Message-ID: <bug-258559-227-1VLcw8Hxg2@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-258559-227@https.bugs.freebsd.org/bugzilla/> References: <bug-258559-227@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D258559 John Baldwin <jhb@FreeBSD.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jhb@FreeBSD.org Assignee|bugs@FreeBSD.org |jhb@FreeBSD.org --- Comment #1 from John Baldwin <jhb@FreeBSD.org> --- Hmm, for me I don't get a crash but instead the system eventually runs out = of swap. This code is indeed buggy and fraught with peril, but it also uses sbrk() which is a deprecated interface on FreeBSD (it's not even present in newer architectures: arm64 and riscv64). [t]csh from the base system shoul= d be built with SYSMALLOC defined so that tcsh uses the malloc() from libc inste= ad of the version from tc.alloc.c. You can verify this by checking for the st= ring ",sm" in the $version variable from tcsh. For example: > echo $version tcsh 6.22.04 (Astron) 2021-04-26 (x86_64-amd-FreeBSD) options wide,nls,dl,al,kan,sm,rh,color,filec Checking in a 12.x jail I had lying around shows ",sm" in $version as well,= and the most recent change to bin/csh/config.h related to this was made in 2007: commit 59dfb2db03584859a57ab6d9519e2de147bf3c4d Author: Mark Peek <mp@FreeBSD.org> Date: Wed May 16 21:22:38 2007 +0000 Work around a vendor issue that was causing the builtin malloc to be used instead of the system malloc. Submitted by: ume Notes: svn path=3D/head/; revision=3D169626 diff --git a/bin/csh/config.h b/bin/csh/config.h index c9b01ef7aa94..0971ffa3faa1 100644 --- a/bin/csh/config.h +++ b/bin/csh/config.h @@ -237,3 +237,6 @@ #ifndef NO_NLS_CATALOGS #define NLS_CATALOGS #endif + +/* Work around a vendor issue where config_f.h is #undef'ing this setting = */ +#define SYSMALLOC diff --git a/bin/csh/config_p.h b/bin/csh/config_p.h index 6de288b387e5..8c29053e3176 100644 --- a/bin/csh/config_p.h +++ b/bin/csh/config_p.h @@ -82,8 +82,6 @@ #if defined(__FreeBSD__) #define NLS_BUGS #define BSD_STYLE_COLORLS -/* we want to use the system malloc when we install as /bin/csh */ -#define SYSMALLOC /* Use LC_MESSAGES locale category to open the message catalog */ #define MCLoadBySet NL_CAT_LOCALE #define BUFSIZE 8192 So I'm surprised you would have a tcsh binary on a 13.0 system that was usi= ng the malloc() implementation from tc.alloc.c rather than the libc allocator. I do think it's true that if you checkout the raw tcsh sources from upstream and build them that it still defaults to using tc.alloc.c. If that is the = way you built tcsh then a patch to tcsh itself to adjust these lines in the upstream config_f.h would work: /* * SYSMALLOC Use the system provided version of malloc and friends. * This can be much slower and no memory statistics will be * provided. */ #if defined(__MACHTEN__) || defined(PURIFY) || defined(MALLOC_TRACE) || defined(_OSD_POSIX) || defined(__MVS__) || defined (__CYGWIN__) || defined(__GLIBC__) || defined(__OpenBSD__) || defined(__APPLE__) || defined (__ANDROID__) # define SYSMALLOC #else # undef SYSMALLOC #endif IMO, upstream tcsh would be better served by just always assuming SYSMALLOC= and dropping its own allocator as it is unlikely to be better than the system malloc on any contemporary OSs. --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-258559-227-1VLcw8Hxg2>