Skip to content

Commit

Permalink
Add lcfs_node_set_symlink_payload
Browse files Browse the repository at this point in the history
Historically we accept any arbitrary data in the dumpfile
for the file size for symlink, and end up ignoring it
ultimately when we write the EROFS. However previously
it was pretty confusing as the in-memory node data
could have a bogus size.

Add an API and use it both in the mkcomposefs path *and*
in the "reread from EROFS path" to perform more consistent
validation for symlink targets. This just wraps the
previous `lcfs_node_set_payload()` API, inheriting its
checks for length.

Signed-off-by: Colin Walters <[email protected]>
  • Loading branch information
cgwalters committed Sep 9, 2024
1 parent 9da2423 commit 941bf23
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 8 deletions.
2 changes: 1 addition & 1 deletion libcomposefs/lcfs-writer-erofs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1746,7 +1746,7 @@ static struct lcfs_node_s *lcfs_build_node_from_image(struct lcfs_image_data *da

memcpy(name_buf, tail_data, file_size);
name_buf[file_size] = 0;
if (lcfs_node_set_payload(node, name_buf) < 0)
if (lcfs_node_set_symlink_payload(node, name_buf) < 0)
return NULL;

} else if (type == S_IFREG && file_size != 0 && erofs_inode_is_flat(cino)) {
Expand Down
29 changes: 28 additions & 1 deletion libcomposefs/lcfs-writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ struct lcfs_node_s *lcfs_load_node_from_file(int dirfd, const char *fname,
return NULL;

target[r] = '\0';
r = lcfs_node_set_payload(ret, target);
r = lcfs_node_set_symlink_payload(ret, target);
if (r < 0)
return NULL;
}
Expand Down Expand Up @@ -917,6 +917,33 @@ int lcfs_node_set_payload(struct lcfs_node_s *node, const char *payload)
return 0;
}

int lcfs_node_set_symlink_payload(struct lcfs_node_s *node, const char *payload)
{
// Caller must have ensured this
assert((node->inode.st_mode & S_IFMT) == S_IFLNK);
// Symlink target must be non-empty
if (payload == NULL || !*payload) {
errno = EINVAL;
return -1;
}
// Call the "raw" API for setting payloads, which also verifies
// maximum length.
if (lcfs_node_set_payload(node, payload) < 0) {
return -1;
}
// We must have set a payload now
assert(node->payload);
// Historically we accept any arbitrary data in the dumpfile
// for the file size for symlink, and end up ignoring it
// ultimately when we write the EROFS. However previously
// it was pretty confusing as the in-memory node data
// could have a bogus size. If somehow the inode state claimed
// something else for size, let's always replace it with the symlink
// length which is canonical.
node->inode.st_size = strlen(node->payload);
return 0;
}

const char *lcfs_node_get_payload(struct lcfs_node_s *node)
{
return node->payload;
Expand Down
2 changes: 2 additions & 0 deletions libcomposefs/lcfs-writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ LCFS_EXTERN const char *lcfs_node_get_xattr_name(struct lcfs_node_s *node,
size_t index);

LCFS_EXTERN int lcfs_node_set_payload(struct lcfs_node_s *node, const char *payload);
LCFS_EXTERN int lcfs_node_set_symlink_payload(struct lcfs_node_s *node,
const char *payload);
LCFS_EXTERN const char *lcfs_node_get_payload(struct lcfs_node_s *node);

LCFS_EXTERN int lcfs_node_set_content(struct lcfs_node_s *node,
Expand Down
15 changes: 9 additions & 6 deletions tools/mkcomposefs.c
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ static char *tree_from_dump_line(dump_info *info, const char *line, size_t line_
if (lcfs_node_try_set_mode(node, mode) < 0) {
return make_error("Invalid mode %o", (unsigned int)mode);
}
unsigned int type = mode & S_IFMT;

err = tree_add_node(info, path, node);
if (err)
Expand Down Expand Up @@ -509,20 +510,22 @@ static char *tree_from_dump_line(dump_info *info, const char *line, size_t line_
if (digest == NULL && err)
return err;

lcfs_node_set_size(node, size);
if (type != S_IFLNK)
lcfs_node_set_size(node, size);
lcfs_node_set_nlink(node, nlink);
lcfs_node_set_uid(node, uid);
lcfs_node_set_gid(node, gid);
lcfs_node_set_rdev64(node, rdev);
lcfs_node_set_mtime(node, &mtime);
// Validate that symlinks are non-empty
if ((mode & S_IFMT) == S_IFLNK) {
if (payload == NULL) {
return make_error("Invalid empty symlink");
if (type == S_IFLNK) {
if (lcfs_node_set_symlink_payload(node, payload) < 0) {
return make_error("Invalid symlink");
}
} else {
if (lcfs_node_set_payload(node, payload) < 0)
return make_error("Invalid payload");
}
if (lcfs_node_set_payload(node, payload) < 0)
return make_error("Invalid payload");
if (content) {
ret = lcfs_node_set_content(node, (uint8_t *)content, size);
if (ret < 0)
Expand Down

0 comments on commit 941bf23

Please sign in to comment.