Bug 243252 - www/firefox core dumps after r522486 (failed to freeze shm)
Summary: www/firefox core dumps after r522486 (failed to freeze shm)
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-gecko (Nobody)
URL:
Keywords:
: 243301 243320 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-01-10 17:26 UTC by jakub_lach
Modified: 2020-11-13 23:02 UTC (History)
8 users (show)

See Also:
bugzilla: maintainer-feedback? (gecko)
jbeich: merge-quarterly+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jakub_lach 2020-01-10 17:26:47 UTC
[952, Main Thread] WARNING: failed to freeze shm: Nieobsługiwana funkcja: file /usr/obj/usr/ports/
Segmentation fault (core dumped)


Removing patch-bug1550891 fixes the problem
Comment 1 Val Packett 2020-01-10 17:44:17 UTC
Do you have a custom kernel without Capsicum support or something? o_0
Comment 2 jakub_lach 2020-01-10 18:25:06 UTC
(In reply to Greg V from comment #1)

I do have stripped kernel, but everything related to share memory is still in place, I'm not sure what dependency introduced in patch could be missing.
Comment 3 Val Packett 2020-01-10 18:36:25 UTC
(In reply to jakub_lach from comment #2)

You probably disabled CAPABILITIES. (CAPABILITY_MODE is probably not required for *this* but will be necessary for Firefox content process sandboxing which I'm working on right now, so I recommend enabling that as well.)

I guess everything in base is still built to handle kernels without CAPABILITIES and there's not that much software out there leveraging it yet, but it's a weird thing to disable. It's public API for programs, it really shouldn't even be optional at this point, the cap_rights_limit(2) manpage doesn't even mention ENOSYS like cap_enter(2) does.
Comment 4 jakub_lach 2020-01-10 19:04:08 UTC
(In reply to Greg V from comment #3)

You are right, the kernel config origin is pre-capiscum and it was not added after it appeared in GENERIC.
Comment 5 jakub_lach 2020-01-11 09:56:40 UTC
(In reply to jakub_lach from comment #4)

Just wanted to confirm that after adding - 

options         CAPABILITY_MODE         # Capsicum capability mode
options         CAPABILITIES            # Capsicum capabilities

Firefox runs. I would recommend adding maybe pkg_message about this dependency, as was the case with POSIX semaphores some time ago iirc. 

Thanks for explanation and kind help!
Comment 6 Jan Beich freebsd_committer freebsd_triage 2020-01-12 20:40:02 UTC
sem(4) was only documented in pkg-message because it was not in GENERIC. For one, removing COMPAT_FREEBSD11 may crash www/firefox as well.
Comment 7 Jan Beich freebsd_committer freebsd_triage 2020-01-13 05:52:35 UTC
*** Bug 243301 has been marked as a duplicate of this bug. ***
Comment 8 commit-hook freebsd_committer freebsd_triage 2020-01-13 06:05:02 UTC
A commit references this bug:

Author: jbeich
Date: Mon Jan 13 06:04:00 UTC 2020
New revision: 522856
URL: https://svnweb.freebsd.org/changeset/ports/522856

Log:
  www/firefox: document Capsicum usage via UPDATING

  PR:		243252
  Reported by:	Jakub Lach
  Analyzed by:	Greg V

Changes:
  head/UPDATING
Comment 9 Jan Beich freebsd_committer freebsd_triage 2020-01-15 13:34:40 UTC
*** Bug 243320 has been marked as a duplicate of this bug. ***
Comment 10 Ivan Rozhuk 2020-01-15 15:38:00 UTC
(In reply to Greg V from comment #3)

> I guess everything in base is still built to handle kernels without CAPABILITIES
and there's not that much software out there leveraging it yet, but it's a weird thing to disable. It's public API for programs, it really shouldn't even be optional at this point, the cap_rights_limit(2) manpage doesn't even mention ENOSYS like cap_enter(2) does.

I drop from kernel all things that not required to boot and that not fundamental subsystems.

Probably better is handle ENOSYS and not require user to add CAPABILITIES into kernel.
Comment 11 Ivan Rozhuk 2020-01-15 15:44:38 UTC
Some thing like:
if (cap_rights_limit(mapped_file_, &rights) != 0 && ENOSYS != errno) {


> Failing to freeze means an untrusted child process has write access.
> The feature is currently used by parent process to pass read-only snapshot
> of preferences to content processes.

It works many years without RO access, nothing happen.
Comment 12 Val Packett 2020-01-15 21:55:37 UTC
(In reply to rozhuk.im from comment #11)

> Probably better is handle ENOSYS and not require user to add CAPABILITIES into kernel.

Sure, we could do that, but preferably only in a downstream (ports) patch that also makes the warning say something like "your kernel config is bad" if it's ENOSYS.

Also how about you get the cap_rights_limit manpage to mention ENOSYS by submitting a patch ;)
Comment 13 Mikhail Teterin freebsd_committer freebsd_triage 2020-01-18 04:59:34 UTC
The following change makes the failure to freeze the shared memory non-fatal -- just as it always was:

Index: files/patch-bug1550891
===================================================================
--- files/patch-bug1550891      (revision 523387)
+++ files/patch-bug1550891      (working copy)
@@ -72,7 +72,7 @@
  #else
    // Generic Unix: shm_open + shm_unlink
    do {
-@@ -275,6 +282,13 @@ bool SharedMemory::Freeze() {
+@@ -275,6 +282,12 @@ bool SharedMemory::Freeze() {
      CHROMIUM_LOG(WARNING) << "failed to freeze shm: " << strerror(errno);
      return false;
    }
@@ -81,7 +81,6 @@
 +  cap_rights_init(&rights, CAP_MMAP_R);
 +  if (cap_rights_limit(mapped_file_, &rights) != 0) {
 +    CHROMIUM_LOG(WARNING) << "failed to freeze shm: " << strerror(errno);
-+    return false;
 +  }
  #else
    DCHECK(frozen_file_ >= 0);
Comment 14 Jan Beich freebsd_committer freebsd_triage 2020-01-18 07:01:25 UTC
(In reply to Mikhail Teterin from comment #13)
Before SHM_ANON was used Firefox reopened the same memory object with read-only permissions. Freezing happened at different time but it was still fatal.

https://searchfox.org/mozilla-central/rev/68b2e0fd4323/ipc/chromium/src/base/shared_memory_posix.cc#231-237
Comment 15 Mikhail Teterin freebsd_committer freebsd_triage 2020-01-18 16:08:07 UTC
(In reply to Jan Beich from comment #14)
> Freezing happened at different time but it was still fatal.

Sorry, Jan, all I know is that Firefox was starting fine for me -- and others -- before patch-bug1550891 landed in files/ 10 days ago, and that it no longer does.

And that my change allows it to start again -- without completely undoing the FreeBSD-specific patch.

People with CAPSICUM enabled will still benefit from the protection it tries to provide.

People without it will still have a working -- if complaining and, possibly, less secure -- browser.
Comment 16 Jan Beich freebsd_committer freebsd_triage 2020-01-18 16:54:56 UTC
(In reply to Mikhail Teterin from comment #15)
> And that my change allows it to start again -- without completely undoing the FreeBSD-specific patch.

If you break freezing promise then it won't be accepted upstream. And I'm not interested in adding more downstream-only patches. There's enough rebase churn even from stuff that *was* submitted upstream but stalled on review for various reasons.

> People with CAPSICUM enabled will still benefit from the protection it tries to provide.

If cap_rights_limit is undesired then on FreeBSD 13 one can use memfd_create(MFD_ALLOW_SEALING). However, older FreeBSD versions would require adjusting ifdefs to ignore SHM_ANON or limiting SHM_ANON to when freezing is not used.
Comment 17 Mikhail Teterin freebsd_committer freebsd_triage 2020-01-20 03:08:18 UTC
(In reply to Jan Beich from comment #16)
> And I'm not interested in adding more downstream-only patches.

I'm confused now... The entire patch-bug1550891 is FreeBSD-specific. If some other, cross-platform, interface was used before, maybe, we ought to go back to it?

> If cap_rights_limit is undesired [...]

I don't know, if is desired or otherwise. All I know, is that I went to update firefox to address a CVE, and got a non-starting executable.

You kindly explained, what's happening -- by pointing me at this bug-report. Clearly, I was not alone with the problem.

I then suggested, the failure to freeze should not be fatal -- and you invited patches. The one-liner I proposed solves the problem -- the freezing is still attempted, but a failure is no longer fatal.

Now, maybe, it is not good enough, but I resent having to recompile the kernel -- and rebooting -- just to upgrade one application. (BTW, what about Spidermonkey and Thunderbird?)
Comment 18 Jan Beich freebsd_committer freebsd_triage 2020-01-20 04:02:43 UTC
(In reply to Mikhail Teterin from comment #17)
> If some other, cross-platform, interface was used before, maybe, we ought to go back to it?

Named shared memory as described in https://pubs.opengroup.org/onlinepubs/9699919799/functions/shm_open.html

Anonymous memory hasn't been standardized despite https://www.austingroupbugs.net/view.php?id=850 being resolved.

> (BTW, what about Spidermonkey and Thunderbird?)

Spidermonkey doesn't use Chromium IPC while Thunderbird predates https://hg.mozilla.org/mozilla-central/rev/16d99c5a8a55.
Comment 19 Val Packett 2020-01-20 17:25:26 UTC
(In reply to Mikhail Teterin from comment #17)
> I resent having to recompile the kernel -- and rebooting -- just to upgrade one application

And I resent supporting non-standard configurations that disable public API that was there in *MINIMAL* for ages and really should not be disable-able anymore ¯\_(ツ)_/¯
Comment 20 Mikhail Teterin freebsd_committer freebsd_triage 2020-01-20 17:54:16 UTC
(In reply to Greg V from comment #19)
> And I resent supporting non-standard configurations

Then don't, Greg. Just don't. Thank you for your past contributions.
Comment 21 Ivan Rozhuk 2020-01-20 18:33:35 UTC
Base system software uses cap_rights_limit(), but respect user:

/usr/src/contrib/dma/dma-mbox-create.c:	if (cap_rights_limit(maildirfd, &rights) < 0 && errno != ENOSYS)

/usr/src/contrib/tcpdump/tcpdump.c:	if (cap_rights_limit(fd, &rights) < 0 && errno != ENOSYS) {


/usr/src/contrib/traceroute/traceroute.c:
#ifdef WITH_CASPER
	cansandbox = true;
#else
	if (nflag)
		cansandbox = true;
	else
		cansandbox = false;
#endif
...


/usr/src/contrib/xz/src/xz/file_io.c:
#ifdef HAVE_CAPSICUM
	// Capsicum needs FreeBSD 10.0 or later.
	cap_rights_t rights;

	if (cap_rights_limit(src_fd, cap_rights_init(&rights,
			CAP_EVENT, CAP_FCNTL, CAP_LOOKUP, CAP_READ, CAP_SEEK)))
		goto error;


/usr/src/crypto/openssh/sandbox-capsicum.c:	if (cap_rights_limit(STDIN_FILENO, &rights) < 0 && errno != ENOSYS)


/usr/src/lib/libutil/pidfile.c:
	if (cap_rights_limit(dirfd,
	    cap_rights_init(&caprights, CAP_UNLINKAT)) < 0 && errno != ENOSYS) {
		goto failed;
	}
...


/usr/src/sbin/hastd/subr.c:
#ifdef HAVE_CAPSICUM
	capsicum = (cap_enter() == 0);
	if (!capsicum) {
		pjdlog_common(LOG_DEBUG, 1, errno,
		    "Unable to sandbox using capsicum");
	} else if (res != NULL) {
		cap_rights_t rights;
		static const unsigned long geomcmds[] = {
		    DIOCGDELETE,
		    DIOCGFLUSH
		};

		PJDLOG_ASSERT(res->hr_role == HAST_ROLE_PRIMARY ||
		    res->hr_role == HAST_ROLE_SECONDARY);

		cap_rights_init(&rights, CAP_FLOCK, CAP_IOCTL, CAP_PREAD,
		    CAP_PWRITE);
		if (cap_rights_limit(res->hr_localfd, &rights) == -1) {
			pjdlog_errno(LOG_ERR,
			    "Unable to limit capability rights on local descriptor");
		}
		if (cap_ioctls_limit(res->hr_localfd, geomcmds,
		    nitems(geomcmds)) == -1) {
			pjdlog_errno(LOG_ERR,
			    "Unable to limit allowed GEOM ioctls");
		}
...


/usr/src/sbin/ping/ping.c:	if (cap_rights_limit(srecv, &rights) < 0 && errno != ENOSYS)


/usr/src/usr.bin/bsdiff/bspatch/bspatch.c:
#ifndef WITHOUT_CAPSICUM
	if (cap_enter() < 0) {
		/* Failed to sandbox, fatal if CAPABILITY_MODE enabled */
		if (errno != ENOSYS)
			err(1, "failed to enter security sandbox");
	} else {
		/* Capsicum Available */
		cap_rights_init(&rights_ro, CAP_READ, CAP_FSTAT, CAP_SEEK);
		cap_rights_init(&rights_wr, CAP_WRITE);
		cap_rights_init(&rights_dir, CAP_UNLINKAT);

		if (cap_rights_limit(fileno(f), &rights_ro) < 0 ||


/usr/src/usr.sbin/iscsid/iscsid.c:
	error = cap_rights_limit(conn->conn_iscsi_fd, &rights);
	if (error != 0 && errno != ENOSYS)
		log_err(1, "cap_rights_limit");


So if you decide that cap_rights_limit() "must have" - ensure that other agree with you and remove WITHO_CAPSICUM/WITHOUT_CAPSICUM from src and base system software before this patch.
Comment 22 Mikhail Teterin freebsd_committer freebsd_triage 2020-01-20 20:02:32 UTC
(In reply to rozhuk.im from comment #21)
> and remove WITHO_CAPSICUM/WITHOUT_CAPSICUM from src and base system
> software before this patch.

That may be overly onerous, but to simply require it for an application to start -- without so much as a warning -- was certainly a blunder, that needs correcting.

Chromium itself, for all its other flaws, has no problem starting on my machine.

Lastly, the patch should not FreeBSD-specific. OpenBSD has the same API too, and, likely, so do the others.
Comment 23 Ivan Rozhuk 2020-04-28 00:14:47 UTC
--- www/firefox/files/patch-bug1550891	(revision 533215)
+++ www/firefox/files/patch-bug1550891	(working copy)
@@ -79,7 +79,7 @@
 +#elif defined(__FreeBSD__)
 +  cap_rights_t rights;
 +  cap_rights_init(&rights, CAP_MMAP_R);
-+  if (cap_rights_limit(mapped_file_, &rights) != 0) {
++  if (cap_rights_limit(mapped_file_, &rights) != 0 && errno != ENOSYS) {
 +    CHROMIUM_LOG(WARNING) << "failed to freeze shm: " << strerror(errno);
 +    return false;
 +  }
Comment 24 Bill Blake 2020-11-03 08:09:34 UTC
This problem persists in 12.2-RELEASE.  I had to come to bugzilla to find out why firefox crashed.  That should not have been necessary.

Fixes: Document Capsicum support as being required for firefox, make the error message mention the need for Capsicum, or remove the dependency on Capsicum.
Comment 25 Ivan Rozhuk 2020-11-13 13:44:12 UTC
@jbeich, in latest FF update www/firefox/files/patch-bug1440203 uses cap_rights_limit() but does not check error code.
But it does not fail if no Capsicum in system.

IMHO this can be closed.
Comment 26 Jan Beich freebsd_committer freebsd_triage 2020-11-13 22:35:42 UTC
(In reply to rozhuk.im from comment #25)
> files/patch-bug1440203 uses cap_rights_limit() but does not check error code

FreeBSD < 13 no longer uses cap_rights_limit() due to missing memfd_create().

> But it does not fail if no Capsicum in system.

What FreeBSD version did you test? 

> IMHO this can be closed.

firefox-esr and thunderbird are still affected. I'll just drop SHM_ANON optimization as stability is more important than performance there.
Comment 27 commit-hook freebsd_committer freebsd_triage 2020-11-13 22:47:29 UTC
A commit references this bug:

Author: jbeich
Date: Fri Nov 13 22:46:34 UTC 2020
New revision: 555054
URL: https://svnweb.freebsd.org/changeset/ports/555054

Log:
  www/firefox-esr: revert r522464 to improve stability

  Upstream adopted memfd_create instead which requires FreeBSD >= 13.
  Having ESR behave differently complicates maintenance, so drop
  SHM_ANON for now.

  PR:		243252

Changes:
  head/mail/thunderbird/Makefile
  head/mail/thunderbird/files/patch-bug1550891
  head/www/firefox-esr/Makefile
  head/www/firefox-esr/files/patch-bug1550891
Comment 28 commit-hook freebsd_committer freebsd_triage 2020-11-13 22:53:31 UTC
A commit references this bug:

Author: jbeich
Date: Fri Nov 13 22:52:57 UTC 2020
New revision: 555056
URL: https://svnweb.freebsd.org/changeset/ports/555056

Log:
  MFH: r555054

  www/firefox-esr: revert r522464 to improve stability

  Upstream adopted memfd_create instead which requires FreeBSD >= 13.
  Having ESR behave differently complicates maintenance, so drop
  SHM_ANON for now.

  PR:		243252
  Approved by:	ports-secteam blanket

Changes:
_U  branches/2020Q4/
  branches/2020Q4/mail/thunderbird/Makefile
  branches/2020Q4/mail/thunderbird/files/patch-bug1550891
  branches/2020Q4/www/firefox-esr/Makefile
  branches/2020Q4/www/firefox-esr/files/patch-bug1550891
Comment 29 Ivan Rozhuk 2020-11-13 23:02:29 UTC
(In reply to Jan Beich from comment #26)

FreeBSD rimwks.local 12.2-STABLE FreeBSD 12.2-STABLE RIM_WKS  amd64