Fix use of removed functions in comm_in

It is better to have swaylock crash immediately and the compositor
show a red screen than to operate in a degraded state where
passwords cannot be checked and the screen cannot be unlocked.

background-image.c

@@ -31,8 +31,12 @@ cairo_surface_t *load_background_image(const char *path) {
 				err->message);
 		return NULL;
 	}
-	image = gdk_cairo_image_surface_create_from_pixbuf(pixbuf);
+	// Correct for embedded image orientation; typical images are not
+	// rotated and will be handled efficiently
+	GdkPixbuf *oriented = gdk_pixbuf_apply_embedded_orientation(pixbuf);
 	g_object_unref(pixbuf);
+	image = gdk_cairo_image_surface_create_from_pixbuf(oriented);
+	g_object_unref(oriented);
 #else
 	image = cairo_image_surface_create_from_png(path);
 #endif // HAVE_GDK_PIXBUF

comm.c

@@ -38,8 +38,8 @@ ssize_t read_comm_request(char **buf_ptr) {
 	return size;
 }
 
-bool write_comm_reply(bool success) {
-	if (write(comm[1][1], &success, sizeof(success)) != sizeof(success)) {
+bool write_comm_reply(struct comm_reply reply) {
+	if (write(comm[1][1], &reply, sizeof(reply)) != sizeof(reply)) {
 		swaylock_log_errno(LOG_ERROR, "failed to write pw check result");
 		return false;
 	}
@@ -95,11 +95,11 @@ out:
 	return result;
 }
 
-bool read_comm_reply(void) {
-	bool result = false;
+struct comm_reply read_comm_reply(void) {
+	struct comm_reply result;
 	if (read(comm[1][0], &result, sizeof(result)) != sizeof(result)) {
 		swaylock_log_errno(LOG_ERROR, "Failed to read pw result");
-		result = false;
+		result.kind = REPLY_AUTH_ERR;
 	}
 	return result;
 }

include/comm.h

@@ -5,13 +5,25 @@
 
 struct swaylock_password;
 
+enum conn_reply_kind {
+	REPLY_SUCCESS,
+	REPLY_AUTH_ERR,
+	REPLY_CONTINUE,
+	REPLY_MSG,
+};
+
+struct comm_reply {
+	enum conn_reply_kind kind;
+	char pam_msg[256];
+};
+
 bool spawn_comm_child(void);
 ssize_t read_comm_request(char **buf_ptr);
-bool write_comm_reply(bool success);
+bool write_comm_reply(struct comm_reply reply);
 // Requests the provided password to be checked. The password is always cleared
 // when the function returns.
 bool write_comm_request(struct swaylock_password *pw);
-bool read_comm_reply(void);
+struct comm_reply read_comm_reply(void);
 // FD to poll for password authentication replies.
 int get_comm_reply_fd(void);
 

include/swaylock.h

@@ -11,6 +11,7 @@
 // Indicator state: status of authentication attempt
 enum auth_state {
 	AUTH_STATE_IDLE, // nothing happening
+	AUTH_STATE_IDLE_MSG, // showing a PAM message
 	AUTH_STATE_VALIDATING, // currently validating password
 	AUTH_STATE_INVALID, // displaying message: password was wrong
 };
@@ -100,6 +101,7 @@ struct swaylock_state {
 	bool run_display, locked;
 	struct ext_session_lock_manager_v1 *ext_session_lock_manager_v1;
 	struct ext_session_lock_v1 *ext_session_lock_v1;
+	char pam_msg[256];
 };
 
 struct swaylock_surface {
@@ -113,12 +115,13 @@ struct swaylock_surface {
 	struct ext_session_lock_surface_v1 *ext_session_lock_surface_v1;
 	struct pool_buffer indicator_buffers[2];
 	bool created;
-	bool frame_pending, dirty;
+	bool dirty;
 	uint32_t width, height;
 	int32_t scale;
 	enum wl_output_subpixel subpixel;
 	char *output_name;
 	struct wl_list link;
+	struct wl_callback *frame;
 	// Dimensions of last wl_buffer committed to background surface
 	int last_buffer_width, last_buffer_height;
 };
@@ -133,12 +136,12 @@ struct swaylock_image {
 
 void swaylock_handle_key(struct swaylock_state *state,
 		xkb_keysym_t keysym, uint32_t codepoint);
-void render_frame_background(struct swaylock_surface *surface);
-void render_frame(struct swaylock_surface *surface);
-void damage_surface(struct swaylock_surface *surface);
+
+void render(struct swaylock_surface *surface);
 void damage_state(struct swaylock_state *state);
 void clear_password_buffer(struct swaylock_password *pw);
 void schedule_auth_idle(struct swaylock_state *state);
+void schedule_auth_idle_msg(struct swaylock_state *state);
 
 void initialize_pw_backend(int argc, char **argv);
 void run_pw_backend_child(void);

main.c

@@ -163,59 +163,19 @@ static void ext_session_lock_surface_v1_handle_configure(void *data,
 	surface->width = width;
 	surface->height = height;
 	ext_session_lock_surface_v1_ack_configure(lock_surface, serial);
-	render_frame_background(surface);
-	render_frame(surface);
+	surface->dirty = true;
+	render(surface);
 }
 
 static const struct ext_session_lock_surface_v1_listener ext_session_lock_surface_v1_listener = {
 	.configure = ext_session_lock_surface_v1_handle_configure,
 };
 
-static const struct wl_callback_listener surface_frame_listener;
-
-static void surface_frame_handle_done(void *data, struct wl_callback *callback,
-		uint32_t time) {
-	struct swaylock_surface *surface = data;
-
-	wl_callback_destroy(callback);
-	surface->frame_pending = false;
-
-	if (surface->dirty) {
-		// Schedule a frame in case the surface is damaged again
-		struct wl_callback *callback = wl_surface_frame(surface->surface);
-		wl_callback_add_listener(callback, &surface_frame_listener, surface);
-		surface->frame_pending = true;
-
-		render_frame(surface);
-		surface->dirty = false;
-	}
-}
-
-static const struct wl_callback_listener surface_frame_listener = {
-	.done = surface_frame_handle_done,
-};
-
-void damage_surface(struct swaylock_surface *surface) {
-	if (surface->width == 0 || surface->height == 0) {
-		// Not yet configured
-		return;
-	}
-
-	surface->dirty = true;
-	if (surface->frame_pending) {
-		return;
-	}
-
-	struct wl_callback *callback = wl_surface_frame(surface->surface);
-	wl_callback_add_listener(callback, &surface_frame_listener, surface);
-	surface->frame_pending = true;
-	wl_surface_commit(surface->surface);
-}
-
 void damage_state(struct swaylock_state *state) {
 	struct swaylock_surface *surface;
 	wl_list_for_each(surface, &state->surfaces, link) {
-		damage_surface(surface);
+		surface->dirty = true;
+		render(surface);
 	}
 }
 
@@ -226,7 +186,8 @@ static void handle_wl_output_geometry(void *data, struct wl_output *wl_output,
 	struct swaylock_surface *surface = data;
 	surface->subpixel = subpixel;
 	if (surface->state->run_display) {
-		damage_surface(surface);
+		surface->dirty = true;
+		render(surface);
 	}
 }
 
@@ -247,7 +208,8 @@ static void handle_wl_output_scale(void *data, struct wl_output *output,
 	struct swaylock_surface *surface = data;
 	surface->scale = factor;
 	if (surface->state->run_display) {
-		damage_surface(surface);
+		surface->dirty = true;
+		render(surface);
 	}
 }
 
@@ -1073,14 +1035,27 @@ static void display_in(int fd, short mask, void *data) {
 }
 
 static void comm_in(int fd, short mask, void *data) {
-	if (read_comm_reply()) {
+	struct comm_reply reply = read_comm_reply();
+	if (reply.kind == REPLY_SUCCESS) {
 		// Authentication succeeded
 		state.run_display = false;
-	} else {
+	} else if (mask & (POLLHUP | POLLERR)) {
+		swaylock_log(LOG_ERROR,	"Password checking subprocess crashed; exiting.");
+		exit(EXIT_FAILURE);
+	} else if (reply.kind == REPLY_AUTH_ERR) {
 		state.auth_state = AUTH_STATE_INVALID;
 		schedule_auth_idle(&state);
 		++state.failed_attempts;
 		damage_state(&state);
+	} else if (reply.kind == REPLY_CONTINUE) {
+		state.auth_state = AUTH_STATE_IDLE;
+		schedule_auth_idle(&state);
+		damage_state(&state);
+	}  else if (reply.kind == REPLY_MSG) {
+		state.auth_state = AUTH_STATE_IDLE_MSG;
+		memcpy(&state.pam_msg[0], &reply.pam_msg[0], 256);
+		schedule_auth_idle_msg(&state);
+		damage_state(&state);
 	}
 }
 

meson.build

@@ -1,7 +1,7 @@
 project(
 	'swaylock',
 	'c',
-	version: '1.7.2',
+	version: '1.8.0',
 	license: 'MIT',
 	meson_version: '>=0.59.0',
 	default_options: [

pam.c

@@ -1,3 +1,4 @@
+#include <security/_pam_types.h>
 #define _POSIX_C_SOURCE 200809L
 #include <pwd.h>
 #include <security/pam_appl.h>
@@ -11,6 +12,7 @@
 #include "swaylock.h"
 
 static char *pw_buf = NULL;
+static bool prompt_send_response = false;
 
 void initialize_pw_backend(int argc, char **argv) {
 	if (getuid() != geteuid() || getgid() != getegid()) {
@@ -38,14 +40,41 @@ static int handle_conversation(int num_msg, const struct pam_message **msg,
 		switch (msg[i]->msg_style) {
 		case PAM_PROMPT_ECHO_OFF:
 		case PAM_PROMPT_ECHO_ON:
-			pam_reply[i].resp = strdup(pw_buf); // PAM clears and frees this
-			if (pam_reply[i].resp == NULL) {
-				swaylock_log(LOG_ERROR, "Allocation failed");
-				return PAM_ABORT;
+		{
+			if (prompt_send_response) {
+				prompt_send_response = false;
+				struct comm_reply reply;
+				reply.kind = REPLY_CONTINUE;
+				if (!write_comm_reply(reply)) {
+					exit(EXIT_FAILURE);
+				}
+			}
+			ssize_t size = read_comm_request(&pw_buf);
+			if (size < 0) {
+				exit(EXIT_FAILURE);
+			} else if (size == 0) {
+				pam_reply[i].resp = NULL;
+			} else {
+				pam_reply[i].resp = strdup(pw_buf); // PAM clears and frees this
+				password_buffer_destroy(pw_buf, size);
+				pw_buf = NULL;
+				if (pam_reply[i].resp == NULL) {
+					swaylock_log(LOG_ERROR, "Allocation failed");
+					return PAM_ABORT;
+				}
 			}
 			break;
+		}
 		case PAM_ERROR_MSG:
 		case PAM_TEXT_INFO:
+			swaylock_log(LOG_DEBUG, "PAM message %s", msg[i]->msg);
+			prompt_send_response = false;
+			struct comm_reply reply;
+			reply.kind = REPLY_MSG;
+			strncpy(&reply.pam_msg[0], msg[i]->msg, 255);
+			if (!write_comm_reply(reply)) {
+				exit(EXIT_FAILURE);
+			}
 			break;
 		}
 	}
@@ -89,16 +118,8 @@ void run_pw_backend_child(void) {
 
 	int pam_status = PAM_SUCCESS;
 	while (1) {
-		ssize_t size = read_comm_request(&pw_buf);
-		if (size < 0) {
-			exit(EXIT_FAILURE);
-		} else if (size == 0) {
-			break;
-		}
-
 		int pam_status = pam_authenticate(auth_handle, 0);
-		password_buffer_destroy(pw_buf, size);
-		pw_buf = NULL;
+		prompt_send_response = false;
 
 		bool success = pam_status == PAM_SUCCESS;
 		if (!success) {
@@ -106,9 +127,22 @@ void run_pw_backend_child(void) {
 				get_pam_auth_error(pam_status));
 		}
 
-		if (!write_comm_reply(success)) {
+		struct comm_reply reply;
+		if (success) {
+			reply.kind = REPLY_SUCCESS;
+		} else {
+			reply.kind = REPLY_AUTH_ERR;
+		}
+
+		if (!write_comm_reply(reply)) {
 			exit(EXIT_FAILURE);
 		}
+
+		if (success) {
+			/* Unsuccessful requests may be queued after a successful one;
+			 * do not process them. */
+			break;
+		}
 	}
 
 	pam_setcred(auth_handle, PAM_REFRESH_CRED);

password.c

@@ -60,6 +60,13 @@ static void set_auth_idle(void *data) {
 	damage_state(state);
 }
 
+static void set_auth_idle_msg(void *data) {
+	struct swaylock_state *state = data;
+	state->auth_idle_timer = NULL;
+	state->auth_state = AUTH_STATE_IDLE_MSG;
+	damage_state(state);
+}
+
 static void schedule_input_idle(struct swaylock_state *state) {
 	if (state->input_idle_timer) {
 		loop_remove_timer(state->eventloop, state->input_idle_timer);
@@ -83,6 +90,14 @@ void schedule_auth_idle(struct swaylock_state *state) {
 		state->eventloop, 3000, set_auth_idle, state);
 }
 
+void schedule_auth_idle_msg(struct swaylock_state *state) {
+	if (state->auth_idle_timer) {
+		loop_remove_timer(state->eventloop, state->auth_idle_timer);
+	}
+	state->auth_idle_timer = loop_add_timer(
+		state->eventloop, 3000, set_auth_idle_msg, state);
+}
+
 static void clear_password(void *data) {
 	struct swaylock_state *state = data;
 	state->clear_password_timer = NULL;
@@ -146,7 +161,7 @@ void swaylock_handle_key(struct swaylock_state *state,
 			state->input_state = INPUT_STATE_CLEAR;
 			cancel_password_clear(state);
 		} else {
-			if (backspace(&state->password)) {
+			if (backspace(&state->password) && state->password.len != 0) {
 				state->input_state = INPUT_STATE_BACKSPACE;
 				schedule_password_clear(state);
 				update_highlight(state);

render.c

@@ -3,6 +3,7 @@
 #include <wayland-client.h>
 #include "cairo.h"
 #include "background-image.h"
+#include "log.h"
 #include "swaylock.h"
 #include "log.h"
 
@@ -33,7 +34,23 @@ static void set_color_for_state(cairo_t *cairo, struct swaylock_state *state,
 	}
 }
 
-void render_frame_background(struct swaylock_surface *surface) {
+static void surface_frame_handle_done(void *data, struct wl_callback *callback,
+		uint32_t time) {
+	struct swaylock_surface *surface = data;
+
+	wl_callback_destroy(callback);
+	surface->frame = NULL;
+
+	render(surface);
+}
+
+static const struct wl_callback_listener surface_frame_listener = {
+	.done = surface_frame_handle_done,
+};
+
+static bool render_frame(struct swaylock_surface *surface);
+
+void render(struct swaylock_surface *surface) {
 	struct swaylock_state *state = surface->state;
 
 	int buffer_width = surface->width * surface->scale;
@@ -42,11 +59,17 @@ void render_frame_background(struct swaylock_surface *surface) {
 		return; // not yet configured
 	}
 
-	wl_surface_set_buffer_scale(surface->surface, surface->scale);
+	if (!surface->dirty || surface->frame) {
+		// Nothing to do or frame already pending
+		return;
+	}
+
+	bool need_destroy = false;
+	struct pool_buffer buffer;
 
 	if (buffer_width != surface->last_buffer_width ||
 			buffer_height != surface->last_buffer_height) {
-		struct pool_buffer buffer;
+		need_destroy = true;
 		if (!create_buffer(state->shm, &buffer, buffer_width, buffer_height,
 				WL_SHM_FORMAT_ARGB8888)) {
 			swaylock_log(LOG_ERROR,
@@ -69,15 +92,23 @@ void render_frame_background(struct swaylock_surface *surface) {
 		cairo_restore(cairo);
 		cairo_identity_matrix(cairo);
 
+		wl_surface_set_buffer_scale(surface->surface, surface->scale);
 		wl_surface_attach(surface->surface, buffer.buffer, 0, 0);
 		wl_surface_damage_buffer(surface->surface, 0, 0, INT32_MAX, INT32_MAX);
-		wl_surface_commit(surface->surface);
-		destroy_buffer(&buffer);
+		need_destroy = true;
 
 		surface->last_buffer_width = buffer_width;
 		surface->last_buffer_height = buffer_height;
-	} else {
-		wl_surface_commit(surface->surface);
+	}
+
+	render_frame(surface);
+	surface->dirty = false;
+	surface->frame = wl_surface_frame(surface->surface);
+	wl_callback_add_listener(surface->frame, &surface_frame_listener, surface);
+	wl_surface_commit(surface->surface);
+
+	if (need_destroy) {
+		destroy_buffer(&buffer);
 	}
 }
 
@@ -99,7 +130,7 @@ static void configure_font_drawing(cairo_t *cairo, struct swaylock_state *state,
 	cairo_font_options_destroy(fo);
 }
 
-void render_frame(struct swaylock_surface *surface) {
+static bool render_frame(struct swaylock_surface *surface) {
 	struct swaylock_state *state = surface->state;
 
 	// First, compute the text that will be drawn, if any, since this
@@ -107,7 +138,8 @@ void render_frame(struct swaylock_surface *surface) {
 
 	char attempts[4]; // like i3lock: count no more than 999
 	char *text = NULL;
-	const char *layout_text = NULL;
+	const char *layout_text = &state->pam_msg[0];
+	swaylock_log(LOG_DEBUG, "RNDR pm %s", layout_text);
 
 	bool draw_indicator = state->args.show_indicator &&
 		(state->auth_state != AUTH_STATE_IDLE ||
@@ -148,11 +180,12 @@ void render_frame(struct swaylock_surface *surface) {
 					++curr_layout;
 				}
 				// will handle invalid index if none are active
-				layout_text = xkb_keymap_layout_get_name(state->xkb.keymap, curr_layout);
+				// layout_text = xkb_keymap_layout_get_name(state->xkb.keymap, curr_layout);
 			}
 		}
 	}
 
+
 	// Compute the size of the buffer needed
 	int arc_radius = state->args.radius * surface->scale;
 	int arc_thickness = state->args.thickness * surface->scale;
@@ -210,7 +243,8 @@ void render_frame(struct swaylock_surface *surface) {
 	struct pool_buffer *buffer = get_next_buffer(state->shm,
 			surface->indicator_buffers, buffer_width, buffer_height);
 	if (buffer == NULL) {
-		return;
+		swaylock_log(LOG_ERROR, "No buffer");
+		return false;
 	}
 
 	// Render the buffer
@@ -352,5 +386,5 @@ void render_frame(struct swaylock_surface *surface) {
 	wl_surface_damage_buffer(surface->child, 0, 0, INT32_MAX, INT32_MAX);
 	wl_surface_commit(surface->child);
 
-	wl_surface_commit(surface->surface);
+	return true;
 }

shadow.c

@@ -1,4 +1,5 @@
 #define _XOPEN_SOURCE // for crypt
+#include <assert.h>
 #include <pwd.h>
 #include <shadow.h>
 #include <stdlib.h>
@@ -14,15 +15,23 @@
 #include "password-buffer.h"
 #include "swaylock.h"
 
+char *encpw = NULL;
+
 void initialize_pw_backend(int argc, char **argv) {
-	if (geteuid() != 0) {
-		swaylock_log(LOG_ERROR,
-				"swaylock needs to be setuid to read /etc/shadow");
+	/* This code runs as root */
+	struct passwd *pwent = getpwuid(getuid());
+	if (!pwent) {
+		swaylock_log_errno(LOG_ERROR, "failed to getpwuid");
 		exit(EXIT_FAILURE);
 	}
-
-	if (!spawn_comm_child()) {
-		exit(EXIT_FAILURE);
+	encpw = pwent->pw_passwd;
+	if (strcmp(encpw, "x") == 0) {
+		struct spwd *swent = getspnam(pwent->pw_name);
+		if (!swent) {
+			swaylock_log_errno(LOG_ERROR, "failed to getspnam");
+			exit(EXIT_FAILURE);
+		}
+		encpw = swent->sp_pwdp;
 	}
 
 	if (setgid(getgid()) != 0) {
@@ -38,40 +47,21 @@ void initialize_pw_backend(int argc, char **argv) {
 			"able to restore it after setuid/setgid)");
 		exit(EXIT_FAILURE);
 	}
-}
-
-void run_pw_backend_child(void) {
-	/* This code runs as root */
-	struct passwd *pwent = getpwuid(getuid());
-	if (!pwent) {
-		swaylock_log_errno(LOG_ERROR, "failed to getpwuid");
-		exit(EXIT_FAILURE);
-	}
-	char *encpw = pwent->pw_passwd;
-	if (strcmp(encpw, "x") == 0) {
-		struct spwd *swent = getspnam(pwent->pw_name);
-		if (!swent) {
-			swaylock_log_errno(LOG_ERROR, "failed to getspnam");
-			exit(EXIT_FAILURE);
-		}
-		encpw = swent->sp_pwdp;
-	}
-
-	/* We don't need any additional logging here because the parent process will
-	 * also fail here and will handle logging for us. */
-	if (setgid(getgid()) != 0) {
-		exit(EXIT_FAILURE);
-	}
-	if (setuid(getuid()) != 0) {
-		exit(EXIT_FAILURE);
-	}
-	if (setuid(0) != -1) {
-		exit(EXIT_FAILURE);
-	}
 
 	/* This code does not run as root */
 	swaylock_log(LOG_DEBUG, "Prepared to authorize user %s", pwent->pw_name);
 
+	if (!spawn_comm_child()) {
+		exit(EXIT_FAILURE);
+	}
+
+	/* Buffer is only used by the child */
+	clear_buffer(encpw, strlen(encpw));
+	encpw = NULL;
+}
+
+void run_pw_backend_child(void) {
+	assert(encpw != NULL);
 	while (1) {
 		char *buf;
 		ssize_t size = read_comm_request(&buf);