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

all: add 'copy' mount option #334

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion libcomposefs/lcfs-mount.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

#include <sys/syscall.h>
#include <sys/mount.h>
#include <sys/mman.h>

#ifdef HAVE_FSCONFIG_CMD_CREATE_LINUX_MOUNT_H
#include <linux/mount.h>
#endif
Expand Down Expand Up @@ -666,6 +668,35 @@ static errint_t lcfs_mount(struct lcfs_mount_state_s *state)
return -EINVAL;
}

static int lcfs_mount_copy_and_seal(int fd) {
int memfd = memfd_create(CFS_MOUNT_SOURCE, MFD_CLOEXEC | MFD_ALLOW_SEALING);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use cleanup_fd int memfd = -1;: here and then return steal_fd(&memfd); (Although i see we need to add a steal_fd for this.

if (memfd < 0)
return -errno;

// in the general case, copy_file_range() to a memfd doesn't work
ssize_t n;
do {
char buffer[1 << 20]; // 1MB
n = read(fd, buffer, sizeof buffer);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to handle EINTR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have copy_file_data_classic()

if (n < 0)
break;
n = write(memfd, buffer, n);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to handle short writes, EINTR and write errors in general. I mean, i'm sure memfd writes don't get i/o errors, but you may be low on memory or whatever.

} while (n > 0);

if (n < 0) {
close(memfd);
return n;
}

if (fcntl(memfd, F_ADD_SEALS, F_SEAL_SEAL | F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_WRITE) < 0) {
int err = -errno;
close(memfd);
return err;
}

return memfd;
}

int lcfs_mount_fd(int fd, const char *mountpoint, struct lcfs_mount_options_s *options)
{
struct lcfs_mount_state_s state = { .mountpoint = mountpoint,
Expand All @@ -680,7 +711,21 @@ int lcfs_mount_fd(int fd, const char *mountpoint, struct lcfs_mount_options_s *o
return -1;
}

int memfd = -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanup_fd

if (options->flags & LCFS_MOUNT_FLAGS_COPY) {
memfd = lcfs_mount_copy_and_seal(fd);
if (memfd < 0) {
return memfd;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather skip the else clause in this kind of "didn't get error" codepath. I.e.:

if (memfd < 0) {
  return memfd:
}
state.fd = memfd;

state.fd = memfd;
}
}

res = lcfs_mount(&state);

if (memfd != -1)
close(memfd);

if (res < 0) {
errno = -res;
return -1;
Expand Down Expand Up @@ -708,10 +753,26 @@ int lcfs_mount_image(const char *path, const char *mountpoint,
if (fd < 0) {
return -1;
}
state.fd = fd;

int memfd = -1;
if (options->flags & LCFS_MOUNT_FLAGS_COPY) {
memfd = lcfs_mount_copy_and_seal(fd);
if (memfd < 0) {
errno = memfd;
return -1;
} else {
state.fd = memfd;
}
} else {
state.fd = fd;
}

res = lcfs_mount(&state);

close(fd);
if (memfd != -1)
close(memfd);

if (res < 0) {
errno = -res;
return -1;
Expand Down
3 changes: 2 additions & 1 deletion libcomposefs/lcfs-mount.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ enum lcfs_mount_flags_t {
LCFS_MOUNT_FLAGS_READONLY = (1 << 1),
LCFS_MOUNT_FLAGS_IDMAP = (1 << 3),
LCFS_MOUNT_FLAGS_TRY_VERITY = (1 << 4),
LCFS_MOUNT_FLAGS_COPY = (1 << 5),

LCFS_MOUNT_FLAGS_MASK = (1 << 5) - 1,
LCFS_MOUNT_FLAGS_MASK = (1 << 6) - 1,
};

struct lcfs_mount_options_s {
Expand Down
5 changes: 5 additions & 0 deletions tools/mountcomposefs.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ int main(int argc, char **argv)
bool opt_verity = false;
bool opt_tryverity = false;
bool opt_ro = false;
bool opt_copy = false;
int opt, fd, res, userns_fd;

while ((opt = getopt(argc, argv, "ht:o:")) != -1) {
Expand Down Expand Up @@ -198,6 +199,8 @@ int main(int argc, char **argv)
opt_ro = false;
} else if (strcmp("ro", key) == 0) {
opt_ro = true;
} else if (strcmp("copy", key) == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add this to the manpage and the usage output.

opt_copy = true;
} else {
errx(EXIT_FAILURE, "Unsupported option %s\n", key);
}
Expand Down Expand Up @@ -248,6 +251,8 @@ int main(int argc, char **argv)
options.flags |= LCFS_MOUNT_FLAGS_TRY_VERITY;
if (opt_ro)
options.flags |= LCFS_MOUNT_FLAGS_READONLY;
if (opt_copy)
options.flags |= LCFS_MOUNT_FLAGS_COPY;

if (opt_idmap != NULL) {
userns_fd = open(opt_idmap, O_RDONLY | O_CLOEXEC | O_NOCTTY);
Expand Down
Loading