colin/bubblewrap
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bind-mount: Include failing path in error message

Browse Source 
Prompted by flatpak/flatpak#4731, in which a misconfigured SMB automount
was failing to be remounted with ENODEV. This would have been easier to
debug if we knew which path could not be remounted.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie
2022-02-08 19:04:55 +00:00
committed by Alexander Larsson
parent 8af578d088
commit 010bde7f37
3 changed files with 59 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
bind-mount.c
View File

@@ -378,7 +378,8 @@ bind_mount_result
bind_mount (int proc_fd, bind_mount (int proc_fd,
const char *src, const char *src,
const char *dest, const char *dest,
bind_option_t options) bind_option_t options,
char **failing_path)
{ {
bool readonly = (options & BIND_READONLY) != 0; bool readonly = (options & BIND_READONLY) != 0;
bool devices = (options & BIND_DEVICES) != 0; bool devices = (options & BIND_DEVICES) != 0;
@@ -406,7 +407,12 @@ bind_mount (int proc_fd,
dest_fd = open (resolved_dest, O_PATH | O_CLOEXEC); dest_fd = open (resolved_dest, O_PATH | O_CLOEXEC);
if (dest_fd < 0) if (dest_fd < 0)
{
if (failing_path != NULL)
*failing_path = steal_pointer (&resolved_dest);
return BIND_MOUNT_ERROR_REOPEN_DEST; return BIND_MOUNT_ERROR_REOPEN_DEST;
}
/* If we are in a case-insensitive filesystem, mountinfo might contain a /* If we are in a case-insensitive filesystem, mountinfo might contain a
* different case combination of the path we requested to mount. * different case combination of the path we requested to mount.
@@ -422,11 +428,19 @@ bind_mount (int proc_fd,
oldroot_dest_proc = get_oldroot_path (dest_proc); oldroot_dest_proc = get_oldroot_path (dest_proc);
kernel_case_combination = readlink_malloc (oldroot_dest_proc); kernel_case_combination = readlink_malloc (oldroot_dest_proc);
if (kernel_case_combination == NULL) if (kernel_case_combination == NULL)
{
if (failing_path != NULL)
*failing_path = steal_pointer (&resolved_dest);
return BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD; return BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD;
}
mount_tab = parse_mountinfo (proc_fd, kernel_case_combination); mount_tab = parse_mountinfo (proc_fd, kernel_case_combination);
if (mount_tab[0].mountpoint == NULL) if (mount_tab[0].mountpoint == NULL)
{ {
if (failing_path != NULL)
*failing_path = steal_pointer (&kernel_case_combination);
errno = EINVAL; errno = EINVAL;
return BIND_MOUNT_ERROR_FIND_DEST_MOUNT; return BIND_MOUNT_ERROR_FIND_DEST_MOUNT;
} }
@@ -437,7 +451,12 @@ bind_mount (int proc_fd,
if (new_flags != current_flags && if (new_flags != current_flags &&
mount ("none", resolved_dest, mount ("none", resolved_dest,
NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0) NULL, MS_SILENT | MS_BIND | MS_REMOUNT | new_flags, NULL) != 0)
{
if (failing_path != NULL)
*failing_path = steal_pointer (&resolved_dest);
return BIND_MOUNT_ERROR_REMOUNT_DEST; return BIND_MOUNT_ERROR_REMOUNT_DEST;
}
/* We need to work around the fact that a bind mount does not apply the flags, so we need to manually /* We need to work around the fact that a bind mount does not apply the flags, so we need to manually
* apply the flags to all submounts in the recursive case. * apply the flags to all submounts in the recursive case.
@@ -456,10 +475,15 @@ bind_mount (int proc_fd,
/* If we can't read the mountpoint we can't remount it, but that should /* If we can't read the mountpoint we can't remount it, but that should
be safe to ignore because its not something the user can access. */ be safe to ignore because its not something the user can access. */
if (errno != EACCES) if (errno != EACCES)
{
if (failing_path != NULL)
*failing_path = xstrdup (mount_tab[i].mountpoint);
return BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT; return BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT;
} }
} }
} }
}
return BIND_MOUNT_SUCCESS; return BIND_MOUNT_SUCCESS;
} }
@@ -469,50 +493,53 @@ bind_mount (int proc_fd,
* If want_errno_p is non-NULL, *want_errno_p is used to indicate whether * If want_errno_p is non-NULL, *want_errno_p is used to indicate whether
* it would make sense to print strerror(saved_errno). * it would make sense to print strerror(saved_errno).
*/ */
const char * static char *
bind_mount_result_to_string (bind_mount_result res, bind_mount_result_to_string (bind_mount_result res,
const char *failing_path,
bool *want_errno_p) bool *want_errno_p)
{ {
const char *string; char *string = NULL;
bool want_errno = TRUE; bool want_errno = TRUE;
switch (res) switch (res)
{ {
case BIND_MOUNT_ERROR_MOUNT: case BIND_MOUNT_ERROR_MOUNT:
string = "Unable to mount source on destination"; string = xstrdup ("Unable to mount source on destination");
break; break;
case BIND_MOUNT_ERROR_REALPATH_DEST: case BIND_MOUNT_ERROR_REALPATH_DEST:
string = "realpath(destination)"; string = xstrdup ("realpath(destination)");
break; break;
case BIND_MOUNT_ERROR_REOPEN_DEST: case BIND_MOUNT_ERROR_REOPEN_DEST:
string = "open(destination, O_PATH)"; string = xasprintf ("open(\"%s\", O_PATH)", failing_path);
break; break;
case BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD: case BIND_MOUNT_ERROR_READLINK_DEST_PROC_FD:
string = "readlink(/proc/self/fd/<destination>)"; string = xasprintf ("readlink(/proc/self/fd/N) for \"%s\"", failing_path);
break; break;
case BIND_MOUNT_ERROR_FIND_DEST_MOUNT: case BIND_MOUNT_ERROR_FIND_DEST_MOUNT:
string = "Unable to find destination in mount table"; string = xasprintf ("Unable to find \"%s\" in mount table", failing_path);
want_errno = FALSE; want_errno = FALSE;
break; break;
case BIND_MOUNT_ERROR_REMOUNT_DEST: case BIND_MOUNT_ERROR_REMOUNT_DEST:
string = "Unable to remount destination with correct flags"; string = xasprintf ("Unable to remount destination \"%s\" with correct flags",
failing_path);
break; break;
case BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT: case BIND_MOUNT_ERROR_REMOUNT_SUBMOUNT:
string = "Unable to remount recursively with correct flags"; string = xasprintf ("Unable to apply mount flags: remount \"%s\"",
failing_path);
break; break;
case BIND_MOUNT_SUCCESS: case BIND_MOUNT_SUCCESS:
string = "Success"; string = xstrdup ("Success");
break; break;
default: default:
string = "(unknown/invalid bind_mount_result)"; string = xstrdup ("(unknown/invalid bind_mount_result)");
break; break;
} }
@@ -525,11 +552,13 @@ bind_mount_result_to_string (bind_mount_result res,
void void
die_with_bind_result (bind_mount_result res, die_with_bind_result (bind_mount_result res,
int saved_errno, int saved_errno,
const char *failing_path,
const char *format, const char *format,
...) ...)
{ {
va_list args; va_list args;
bool want_errno = TRUE; bool want_errno = TRUE;
char *message;
fprintf (stderr, "bwrap: "); fprintf (stderr, "bwrap: ");
@@ -537,7 +566,9 @@ die_with_bind_result (bind_mount_result res,
vfprintf (stderr, format, args); vfprintf (stderr, format, args);
va_end (args); va_end (args);
fprintf (stderr, ": %s", bind_mount_result_to_string (res, &want_errno)); message = bind_mount_result_to_string (res, failing_path, &want_errno);
fprintf (stderr, ": %s", message);
/* message is leaked, but we're exiting unsuccessfully anyway, so ignore */
if (want_errno) if (want_errno)
fprintf (stderr, ": %s", strerror (saved_errno)); fprintf (stderr, ": %s", strerror (saved_errno));

9
bind-mount.h
View File

@@ -42,14 +42,13 @@ typedef enum
bind_mount_result bind_mount (int proc_fd, bind_mount_result bind_mount (int proc_fd,
const char *src, const char *src,
const char *dest, const char *dest,
bind_option_t options); bind_option_t options,
char **failing_path);
const char *bind_mount_result_to_string (bind_mount_result res,
bool *want_errno);
void die_with_bind_result (bind_mount_result res, void die_with_bind_result (bind_mount_result res,
int saved_errno, int saved_errno,
const char *failing_path,
const char *format, const char *format,
...) ...)
__attribute__((__noreturn__)) __attribute__((__noreturn__))
__attribute__((format (printf, 3, 4))); __attribute__((format (printf, 4, 5)));

11
bubblewrap.c
View File

@@ -1005,6 +1005,7 @@ privileged_op (int privileged_op_socket,
const char *arg2) const char *arg2)
{ {
bind_mount_result bind_result; bind_mount_result bind_result;
char *failing_path = NULL;
if (privileged_op_socket != -1) if (privileged_op_socket != -1)
{ {
@@ -1070,23 +1071,25 @@ privileged_op (int privileged_op_socket,
break; break;
case PRIV_SEP_OP_REMOUNT_RO_NO_RECURSIVE: case PRIV_SEP_OP_REMOUNT_RO_NO_RECURSIVE:
bind_result = bind_mount (proc_fd, NULL, arg2, BIND_READONLY); bind_result = bind_mount (proc_fd, NULL, arg2, BIND_READONLY, &failing_path);
if (bind_result != BIND_MOUNT_SUCCESS) if (bind_result != BIND_MOUNT_SUCCESS)
die_with_bind_result (bind_result, errno, die_with_bind_result (bind_result, errno, failing_path,
"Can't remount readonly on %s", arg2); "Can't remount readonly on %s", arg2);
assert (failing_path == NULL); /* otherwise we would have died */
break; break;
case PRIV_SEP_OP_BIND_MOUNT: case PRIV_SEP_OP_BIND_MOUNT:
/* We always bind directories recursively, otherwise this would let us /* We always bind directories recursively, otherwise this would let us
access files that are otherwise covered on the host */ access files that are otherwise covered on the host */
bind_result = bind_mount (proc_fd, arg1, arg2, BIND_RECURSIVE | flags); bind_result = bind_mount (proc_fd, arg1, arg2, BIND_RECURSIVE | flags, &failing_path);
if (bind_result != BIND_MOUNT_SUCCESS) if (bind_result != BIND_MOUNT_SUCCESS)
die_with_bind_result (bind_result, errno, die_with_bind_result (bind_result, errno, failing_path,
"Can't bind mount %s on %s", arg1, arg2); "Can't bind mount %s on %s", arg1, arg2);
assert (failing_path == NULL); /* otherwise we would have died */
break; break;
case PRIV_SEP_OP_PROC_MOUNT: case PRIV_SEP_OP_PROC_MOUNT: