Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pledge(2) support on OpenBSD #217

Merged
merged 1 commit into from Jan 7, 2019
Merged

Conversation

@worr
Copy link
Contributor

@worr worr commented Sep 21, 2018

On other platforms, this will be a no-op. Fixes #164

I'm currently running it on a recent amd64 snapshot.

@gonzalo-
Copy link

@gonzalo- gonzalo- commented Oct 5, 2018

Hello, please can you check this one?

https://marc.info/?l=openbsd-ports&m=153797184227204&w=2

After that, looks safe to switch to pledge(2) so far.

On other platforms, this will be a no-op. Fixes #164.
@worr worr force-pushed the worr:feature/pledge branch from 5066c93 to a3dbe9d Oct 7, 2018
@worr
Copy link
Contributor Author

@worr worr commented Oct 7, 2018

Hmm, I did I not run into the issues that Björn did.

That said, I've updated it with the additional pledge that they proposed, as well as moved the call to pledge(2) before the call to 1getpwuid(3) and getuid(3).

@bket
Copy link
Contributor

@bket bket commented Nov 6, 2018

@worr, I ran into a pledge() related issue after doing a rm -rf ~/.cache/fontconfig and restarting xenodm. It seems that spectrwm assumes that ~/.cache/fontconfig, including its contents, exists. If the latter is missing spectrwm tries to create it, which is hindered by pledge(). For reference purposes, I'm using OpenBSD current on amd64.

The issue is reproducible using rm -rf ~/.cache/fontconfig; doas pkill X.

Workaround is to have something like spectrwm || cwm in ~/.xsession. Assuming that ~/.cache/fontconfig does not exist:

  1. spectrwm fails because of pledge()
  2. instead cwm is run, which creates the missing ~/.cache/fontconfig, and its contents
  3. upon a restart, spectrwm starts successfully

After some digging I think the diff below (using your pull request as starting point) fixes the issue:

diff --git spectrwm.c spectrwm.c
index d8d8c62..4d8e903 100644
--- spectrwm.c
+++ spectrwm.c
@@ -3883,9 +3883,6 @@ spawn(int ws_idx, union arg *args, bool close_fd)
 	if (args == NULL || args->argv[0] == NULL)
 		return;
 
-	if (pledge("stdio proc exec", NULL) == -1)
-		err(1, "pledge");
-
 	DNPRINTF(SWM_D_MISC, "%s\n", args->argv[0]);
 
 	close(xcb_get_file_descriptor(conn));
@@ -12475,7 +12472,8 @@ main(int argc, char *argv[])
 	if (setlocale(LC_CTYPE, "") == NULL || setlocale(LC_TIME, "") == NULL)
 		warnx("no locale support");
 
-	if (pledge("stdio rpath proc exec getpw dns unix", NULL) == -1)
+	if (pledge("stdio proc exec cpath rpath wpath fattr getpw dns inet "
+	    "unix", NULL) == -1)
 		err(1, "pledge");
 
 	/* handle some signals */
@@ -12495,6 +12493,10 @@ main(int argc, char *argv[])
 	if ((display = XOpenDisplay(0)) == NULL)
 		errx(1, "can not open display");
 
+	if (pledge("stdio proc exec cpath rpath wpath fattr getpw",
+	    NULL) == -1)
+		err(1, "pledge");
+
 	conn = XGetXCBConnection(display);
 	if (xcb_connection_has_error(conn))
 		errx(1, "can not get XCB connection");
@@ -12504,9 +12506,6 @@ main(int argc, char *argv[])
 	xcb_prefetch_extension_data(conn, &xcb_randr_id);
 	xfd = xcb_get_file_descriptor(conn);
 
-	if (pledge("stdio rpath proc exec getpw", NULL) == -1)
-		err(1, "pledge");
-
 	/* look for local and global conf file */
 	pwd = getpwuid(getuid());
 	if (pwd == NULL)
@@ -12591,6 +12590,9 @@ noconfig:
 	if (cfile)
 		conf_load(cfile, SWM_CONF_DEFAULT);
 
+	if (pledge("stdio proc exec cpath rpath wpath fattr", NULL) == -1)
+		err(1, "pledge");
+
 	validate_spawns();
 
 	if (getenv("SWM_STARTED") == NULL)
@@ -12602,6 +12604,9 @@ noconfig:
 		TAILQ_FOREACH(r, &screens[i].rl, entry)
 			bar_setup(r);
 
+	if (pledge("stdio proc exec", NULL) == -1)
+		err(1, "pledge");
+
 	/* Manage existing windows. */
 	grab_windows();
 
@@ -12685,6 +12690,9 @@ noconfig:
 		xcb_flush(conn);
 	}
 done:
+	if (pledge("stdio proc", NULL) == -1)
+		err(1, "pledge");
+
 	shutdown_cleanup();
 
 	return (0);
@dajohi
dajohi approved these changes Jan 7, 2019
Copy link
Contributor

@dajohi dajohi left a comment

ok

@dajohi dajohi merged commit d619f90 into conformal:master Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants