Skip to content

Commit

Permalink
Merge pull request containers#219 from alexlarsson/error-cleanup
Browse files Browse the repository at this point in the history 
Various cleanups and fixes of error handling
  • Loading branch information
@alexlarsson
alexlarsson committed Oct 12, 2023
2 parents 061f459 + 4bef6c6 commit f5be841
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 47 deletions.
3 changes: 3 additions & 0 deletions libcomposefs/lcfs-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
#include "lcfs-fsverity.h"
#include "hash.h"

/* This is used for (internal) functions that return zero or -errno, functions that set errno return int */
typedef int errint_t;

/* When using LCFS_BUILD_INLINE_SMALL in lcfs_load_node_from_file() inline files below this size
* We pick 64 which is the size of a sha256 digest that would otherwise be used as a redirect
* xattr, so the inlined file is smaller.
Expand Down
66 changes: 36 additions & 30 deletions libcomposefs/lcfs-mount.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ static char *escape_mount_option(const char *str)
return res;
}

static int lcfs_validate_mount_options(struct lcfs_mount_state_s *state)
static errint_t lcfs_validate_mount_options(struct lcfs_mount_state_s *state)
{
struct lcfs_mount_options_s *options = state->options;

Expand Down Expand Up @@ -200,7 +200,7 @@ static int lcfs_validate_mount_options(struct lcfs_mount_state_s *state)
return 0;
}

static int lcfs_validate_verity_fd(struct lcfs_mount_state_s *state)
static errint_t lcfs_validate_verity_fd(struct lcfs_mount_state_s *state)
{
struct {
struct fsverity_digest fsv;
Expand All @@ -225,7 +225,7 @@ static int lcfs_validate_verity_fd(struct lcfs_mount_state_s *state)
return 0;
}

static int setup_loopback(int fd, const char *image_path, char *loopname)
static errint_t setup_loopback(int fd, const char *image_path, char *loopname)
{
struct loop_config loopconfig = { 0 };
int loopctlfd, loopfd;
Expand Down Expand Up @@ -296,7 +296,7 @@ static char *compute_lower(const char *imagemount,
return lower;
}

static int lcfs_mount_ovl_legacy(struct lcfs_mount_state_s *state, char *imagemount)
static errint_t lcfs_mount_ovl_legacy(struct lcfs_mount_state_s *state, char *imagemount)
{
struct lcfs_mount_options_s *options = state->options;

Expand Down Expand Up @@ -351,21 +351,22 @@ static int lcfs_mount_ovl_legacy(struct lcfs_mount_state_s *state, char *imagemo
if (lowerdir_target == lowerdir_1)
mount_flags |= MS_SILENT;

errint_t err = 0;
res = mount("overlay", state->mountpoint, "overlay", mount_flags,
overlay_options);
if (res != 0) {
res = -errno;
err = -errno;
}

if (res == -EINVAL && lowerdir_target == lowerdir_1) {
if (err == -EINVAL && lowerdir_target == lowerdir_1) {
lowerdir_target = lowerdir_2;
goto retry;
}

return res;
return err;
}

static int lcfs_mount_ovl(struct lcfs_mount_state_s *state, char *imagemount)
static errint_t lcfs_mount_ovl(struct lcfs_mount_state_s *state, char *imagemount)
{
#ifdef HAVE_NEW_MOUNT_API
struct lcfs_mount_options_s *options = state->options;
Expand Down Expand Up @@ -476,8 +477,9 @@ static int lcfs_mount_ovl(struct lcfs_mount_state_s *state, char *imagemount)
#endif
}

static int lcfs_mount_erofs(const char *source, const char *target,
uint32_t image_flags, struct lcfs_mount_state_s *state)
static errint_t lcfs_mount_erofs(const char *source, const char *target,
uint32_t image_flags,
struct lcfs_mount_state_s *state)
{
bool image_has_acls = (image_flags & LCFS_EROFS_FLAGS_HAS_ACL) != 0;
bool use_idmap = (state->options->flags & LCFS_MOUNT_FLAGS_IDMAP) != 0;
Expand Down Expand Up @@ -553,16 +555,17 @@ static int lcfs_mount_erofs(const char *source, const char *target,

#define HEADER_SIZE sizeof(struct lcfs_erofs_header_s)

static int lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state,
struct lcfs_erofs_header_s *header)
static errint_t lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state,
struct lcfs_erofs_header_s *header)
{
struct lcfs_mount_options_s *options = state->options;
uint32_t image_flags;
char imagemountbuf[] = "/tmp/.composefs.XXXXXX";
char *imagemount;
bool created_tmpdir = false;
char loopname[PATH_MAX];
int res, errsv;
int errsv;
errint_t err;
int loopfd;

image_flags = lcfs_u32_from_file(header->flags);
Expand All @@ -583,37 +586,38 @@ static int lcfs_mount_erofs_ovl(struct lcfs_mount_state_s *state,
created_tmpdir = true;
}

res = lcfs_mount_erofs(loopname, imagemount, image_flags, state);
err = lcfs_mount_erofs(loopname, imagemount, image_flags, state);
close(loopfd);
if (res < 0) {
if (err < 0) {
rmdir(imagemount);
return res;
return err;
}

/* We use the legacy API to mount overlayfs, because the new API doesn't allow use
* to pass in escaped directory names
*/
res = lcfs_mount_ovl(state, imagemount);
if (res == -ENOSYS)
res = lcfs_mount_ovl_legacy(state, imagemount);
err = lcfs_mount_ovl(state, imagemount);
if (err == -ENOSYS)
err = lcfs_mount_ovl_legacy(state, imagemount);

umount2(imagemount, MNT_DETACH);
if (created_tmpdir) {
rmdir(imagemount);
}

return res;
return err;
}

static int lcfs_mount(struct lcfs_mount_state_s *state)
static errint_t lcfs_mount(struct lcfs_mount_state_s *state)
{
uint8_t header_data[HEADER_SIZE];
struct lcfs_erofs_header_s *erofs_header;
int err;
int res;

res = lcfs_validate_verity_fd(state);
if (res < 0)
return res;
err = lcfs_validate_verity_fd(state);
if (err < 0)
return err;

res = pread(state->fd, &header_data, HEADER_SIZE, 0);
if (res < 0)
Expand All @@ -631,11 +635,12 @@ int lcfs_mount_fd(int fd, const char *mountpoint, struct lcfs_mount_options_s *o
struct lcfs_mount_state_s state = { .mountpoint = mountpoint,
.options = options,
.fd = fd };
errint_t err;
int res;

res = lcfs_validate_mount_options(&state);
if (res < 0) {
errno = -res;
err = lcfs_validate_mount_options(&state);
if (err < 0) {
errno = -err;
return -1;
}

Expand All @@ -654,11 +659,12 @@ int lcfs_mount_image(const char *path, const char *mountpoint,
.mountpoint = mountpoint,
.options = options,
.fd = -1 };
errint_t err;
int fd, res;

res = lcfs_validate_mount_options(&state);
if (res < 0) {
errno = -res;
err = lcfs_validate_mount_options(&state);
if (err < 0) {
errno = -err;
return -1;
}

Expand Down
9 changes: 6 additions & 3 deletions libcomposefs/lcfs-writer-erofs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ static int compute_erofs_shared_xattrs(struct lcfs_ctx_s *ctx)
xattr_hash = hash_initialize(n_files, NULL, xattrs_ht_hasher,
xattrs_ht_comparator, free);
if (xattr_hash == NULL) {
errno = ENOMEM;
return -1;
}

Expand Down Expand Up @@ -813,8 +814,10 @@ static int write_erofs_inode_data(struct lcfs_ctx_s *ctx, struct lcfs_node_s *no
compute_erofs_xattr_counts(node, &n_shared_xattrs, &unshared_xattrs_size);
xattr_size = xattr_erofs_inode_size(n_shared_xattrs, unshared_xattrs_size);
xattr_icount = xattr_erofs_icount(xattr_size);
if (xattr_icount > UINT16_MAX)
return -EINVAL;
if (xattr_icount > UINT16_MAX) {
errno = EINVAL;
return -1;
}

version = node->erofs_compact ? 0 : 1;
datalayout = (node->erofs_tailsize > 0) ? EROFS_INODE_FLAT_INLINE :
Expand Down Expand Up @@ -1523,7 +1526,7 @@ static int lcfs_build_node_erofs_xattr(struct lcfs_node_s *node, uint8_t name_in
}
node->payload = strndup(value, value_size);
if (node->payload == NULL) {
errno = EINVAL;
errno = ENOMEM;
return -1;
}
}
Expand Down
3 changes: 2 additions & 1 deletion libcomposefs/lcfs-writer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ int lcfs_clone_root(struct lcfs_ctx_s *ctx)

clone = lcfs_node_clone_deep(ctx->root);
if (clone == NULL) {
errno = EINVAL;
errno = ENOMEM;
return -1;
}

Expand Down Expand Up @@ -364,6 +364,7 @@ int lcfs_write_to(struct lcfs_node_s *root, struct lcfs_write_options_s *options
}

if (res < 0) {
PROTECT_ERRNO;
lcfs_close(ctx);
return res;
}
Expand Down
20 changes: 10 additions & 10 deletions tools/cfs-fuse.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,8 @@ static const char *cfs_xattr_rewrite(int name_index, const char *name,
return name;
}

static int cfs_listxattr_element(const struct erofs_xattr_entry *entry,
char *buf, size_t *buf_size, size_t max_buf_size)
static errint_t cfs_listxattr_element(const struct erofs_xattr_entry *entry,
char *buf, size_t *buf_size, size_t max_buf_size)
{
const char *name = (const char *)entry + sizeof(struct erofs_xattr_entry);
uint8_t name_len = entry->e_name_len;
Expand Down Expand Up @@ -747,7 +747,7 @@ static void cfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t max_size)
const uint8_t *xattrs_inline;
const uint8_t *xattrs_start;
const uint8_t *xattrs_end;
int res;
errint_t err;

if (cino == NULL) {
fuse_reply_err(req, ENOENT);
Expand Down Expand Up @@ -788,9 +788,9 @@ static void cfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t max_size)
size_t el_size = round_up(
sizeof(struct erofs_xattr_entry) + name_len + value_size, 4);

res = cfs_listxattr_element(entry, buf, &buf_size, max_size);
if (res < 0) {
fuse_reply_err(req, -res);
err = cfs_listxattr_element(entry, buf, &buf_size, max_size);
if (err < 0) {
fuse_reply_err(req, -err);
return;
}
xattrs_inline += el_size;
Expand All @@ -802,9 +802,9 @@ static void cfs_listxattr(fuse_req_t req, fuse_ino_t ino, size_t max_size)
const struct erofs_xattr_entry *entry =
(const struct erofs_xattr_entry *)(erofs_xattrdata + idx * 4);

res = cfs_listxattr_element(entry, buf, &buf_size, max_size);
if (res < 0) {
fuse_reply_err(req, -res);
err = cfs_listxattr_element(entry, buf, &buf_size, max_size);
if (err < 0) {
fuse_reply_err(req, -err);
return;
}
}
Expand Down Expand Up @@ -978,7 +978,7 @@ static void cfs_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
fd = openat(basedir_fd, redirect,
O_CLOEXEC | O_NOCTTY | O_NOFOLLOW | O_RDONLY, 0);
if (fd < 0) {
fuse_reply_err(req, -errno);
fuse_reply_err(req, errno);
return;
}

Expand Down
7 changes: 4 additions & 3 deletions tools/mkcomposefs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ static int join_paths(char **out, const char *path1, const char *path2)
return asprintf(out, "%.*s%s%s", len, path1, sep, path2);
}

static int enable_verity(int fd)
static errint_t enable_verity(int fd)
{
struct fsverity_enable_arg arg = {};

Expand Down Expand Up @@ -200,6 +200,7 @@ static int copy_file_with_dirs_if_needed(const char *src, const char *dst_base,
cleanup_free char *pathbuf = NULL;
cleanup_unlink_free char *tmppath = NULL;
int ret, res;
errint_t err;
cleanup_fd int sfd = -1;
cleanup_fd int dfd = -1;
struct stat statbuf;
Expand Down Expand Up @@ -258,8 +259,8 @@ static int copy_file_with_dirs_if_needed(const char *src, const char *dst_base,
}

if (fstat(dfd, &statbuf) == 0) {
res = enable_verity(dfd);
if (res < 0) {
err = enable_verity(dfd);
if (err < 0) {
/* Ignore errors, we're only trying to enable it */
}
}
Expand Down

0 comments on commit f5be841

Please sign in to comment.