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

Skip directories and symlinks when mounting libraries #282

Merged
merged 1 commit into from
Sep 17, 2024
Merged
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
38 changes: 21 additions & 17 deletions src/nvc_mount.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ static char **mount_files(struct error *, const char *, const struct nvc_contain
static char **mount_driverstore_files(struct error *, const char *, const struct nvc_container *, const char *, const char *[], size_t);
static char *mount_directory(struct error *, const char *, const struct nvc_container *, const char *);
static char *mount_firmware(struct error *, const char *, const struct nvc_container *, const char *);
static char *mount_in_root(struct error *err, const char *src, const char *rootfs, const char *path, uid_t uid, uid_t gid, unsigned long mountflags);
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, we shouldn't have the variable names here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider renaming to mount_at_with_flags()

static char *mount_with_flags(struct error *, const char *, const char *, uid_t, uid_t, unsigned long);
static char *mount_device(struct error *, const char *, const struct nvc_container *, const struct nvc_device_node *);
static char *mount_ipc(struct error *, const char *, const struct nvc_container *, const char *);
Expand All @@ -49,12 +50,9 @@ static char *
mount_directory(struct error *err, const char *root, const struct nvc_container *cnt, const char *dir)
{
char src[PATH_MAX];
char dst[PATH_MAX];
if (path_join(err, src, root, dir) < 0)
return (NULL);
Comment on lines 53 to 54
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider changing this to path_resolve_full() to be symmetrical with what we now do in mount_firmware().

if (path_resolve_full(err, dst, cnt->cfg.rootfs, dir) < 0)
return (NULL);
return mount_with_flags(err, src, dst, cnt->uid, cnt->gid, MS_NOSUID|MS_NOEXEC);
return mount_in_root(err, src, cnt->cfg.rootfs, dir, cnt->uid, cnt->gid, MS_NOSUID|MS_NOEXEC);
}

// mount_firmware mounts the specified firmware file. The path specified is the container path and is resolved
Expand All @@ -63,12 +61,19 @@ static char *
mount_firmware(struct error *err, const char *root, const struct nvc_container *cnt, const char *container_path)
{
char src[PATH_MAX];
char dst[PATH_MAX];
if (path_resolve_full(err, src, root, container_path) < 0)
return (NULL);
if (path_join(err, dst, cnt->cfg.rootfs, container_path) < 0)
return mount_in_root(err, src, cnt->cfg.rootfs, container_path, cnt->uid, cnt->gid, MS_RDONLY|MS_NODEV|MS_NOSUID);
}

// mount_in_root bind mounts the specified src to the specified location in a root.
// If the destination resolves outside of the root an error is raised.
static char *
mount_in_root(struct error *err, const char *src, const char *rootfs, const char *path, uid_t uid, uid_t gid, unsigned long mountflags) {
char dst[PATH_MAX];
if (path_resolve_full(err, dst, rootfs, path) < 0)
return (NULL);
return mount_with_flags(err, src, dst, cnt->uid, cnt->gid, MS_RDONLY|MS_NODEV|MS_NOSUID);
return mount_with_flags(err, src, dst, uid, gid, mountflags);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we can, let's just fold the logic for mount_with_flags() in here.

}

// mount_with_flags bind mounts the specified src to the specified dst with the specified mount flags
Expand Down Expand Up @@ -111,6 +116,8 @@ mount_files(struct error *err, const char *root, const struct nvc_container *cnt
return (NULL);
if (file_create(err, dst, NULL, cnt->uid, cnt->gid, MODE_DIR(0755)) < 0)
return (NULL);
if (path_new(err, dst, dir) < 0)
return (NULL);
src_end = src + strlen(src);
dst_end = dst + strlen(dst);

Expand All @@ -124,19 +131,16 @@ mount_files(struct error *err, const char *root, const struct nvc_container *cnt
continue;
if (path_append(err, src, paths[i]) < 0)
goto fail;
if (path_append(err, dst, file) < 0)
goto fail;
if (file_mode(err, src, &mode) < 0)
if (file_mode_nofollow(err, src, &mode) < 0)
goto fail;
if (file_create(err, dst, NULL, cnt->uid, cnt->gid, mode) < 0)
// If we encounter resolved directories or symlinks here, we raise an error.
if (S_ISDIR(mode) || S_ISLNK(mode)) {
error_setx(err, "unexpected source file mode %o for %s", mode, paths[i]);
goto fail;

log_infof("mounting %s at %s", src, dst);
if (xmount(err, src, dst, NULL, MS_BIND, NULL) < 0)
goto fail;
if (xmount(err, NULL, dst, NULL, MS_BIND|MS_REMOUNT | MS_RDONLY|MS_NODEV|MS_NOSUID, NULL) < 0)
}
if (path_append(err, dst, file) < 0)
goto fail;
if ((*ptr++ = xstrdup(err, dst)) == NULL)
if ((*ptr++ = mount_in_root(err, src, cnt->cfg.rootfs, dst, cnt->uid, cnt->gid, MS_RDONLY|MS_NODEV|MS_NOSUID)) == NULL)
goto fail;
*src_end = '\0';
*dst_end = '\0';
Expand Down
18 changes: 16 additions & 2 deletions src/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,9 @@ file_create(struct error *err, const char *path, const char *data, uid_t uid, gi
int rv = -1;

// We check whether the file already exists with the required mode and skip the creation.
if (data == NULL && file_mode(err, path, &perm) == 0) {
if (data == NULL && file_mode_nofollow(err, path, &perm) == 0) {
if (perm == mode) {
log_errf("The path %s already exists with the required mode; skipping create", path);
log_warnf("The path %s already exists with the required mode; skipping create", path);
return (0);
}
}
Expand Down Expand Up @@ -677,6 +677,20 @@ file_mode(struct error *err, const char *path, mode_t *mode)
return (0);
}

// file_mode_nofollow implements the same functionality as file_mode except that
// in that case of a symlink, the file is not followed and the mode of the
// original file is returned.
int
file_mode_nofollow(struct error *err, const char *path, mode_t *mode)
{
struct stat s;

if (xlstat(err, path, &s) < 0)
return (-1);
*mode = s.st_mode;
return (0);
}

int
file_read_line(struct error *err, const char *path, char *buf, size_t size)
{
Expand Down
1 change: 1 addition & 0 deletions src/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ int file_remove(struct error *, const char *);
int file_exists(struct error *, const char *);
int file_exists_at(struct error *, const char *, const char *);
int file_mode(struct error *, const char *, mode_t *);
int file_mode_nofollow(struct error *, const char *, mode_t *);
int file_read_line(struct error *, const char *, char *, size_t);
int file_read_text(struct error *, const char *, char **);
int file_read_uint32(struct error *, const char *, uint32_t *);
Expand Down
11 changes: 11 additions & 0 deletions src/xfuncs.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ static inline void xclose(int);
static inline int xopen(struct error *, const char *, int);
static inline void *xcalloc(struct error *, size_t, size_t);
static inline int xstat(struct error *, const char *, struct stat *);
static inline int xlstat(struct error *, const char *, struct stat *);
static inline FILE *xfopen(struct error *, const char *, const char *);
static inline char *xstrdup(struct error *, const char *);
static inline int xasprintf(struct error *, char **, const char *, ...)
Expand Down Expand Up @@ -74,6 +75,16 @@ xstat(struct error *err, const char *path, struct stat *buf)
return (rv);
}

static inline int
xlstat(struct error *err, const char *path, struct stat *buf)
{
int rv;

if ((rv = lstat(path, buf)) < 0)
error_set(err, "lstat failed: %s", path);
return (rv);
}

static inline FILE *
xfopen(struct error *err, const char *path, const char *mode)
{
Expand Down