Skip to content

mount_masked_dir: double close of private_data->maskdir_fd on the move_mount failure path #2171

Description

@eriksjolund

When I discussed with Claude Opus 5 in #2170 , Claude Opus 5 mentioned 3 other problems. Here is one of them.
Copy-paste of the text from the Claude Opus 5 web interface:

mount_masked_dir: double close of maskdir_fd when fs_move_mount_to() fails
Title: mount_masked_dir: double close of private_data->maskdir_fd on the move_mount failure path
Body:
Found by code inspection; present on main and at [86e7e3e](https://github.com/containers/crun/blob/86e7e3eaf8e8d15e6e9983faddeffd0ea0771a94/src/libcrun/linux.c#L1143-L1206).
In mount_masked_dir(), mountfd is declared cleanup_close:
https://github.com/containers/crun/blob/86e7e3eaf8e8d15e6e9983faddeffd0ea0771a94/src/libcrun/linux.c#L1147
and is then assigned a descriptor that is already owned by private_data:
https://github.com/containers/crun/blob/86e7e3eaf8e8d15e6e9983faddeffd0ea0771a94/src/libcrun/linux.c#L1163
When fs_move_mount_to() fails, the code closes that descriptor explicitly and clears the struct field, but does not clear mountfd:
https://github.com/containers/crun/blob/86e7e3eaf8e8d15e6e9983faddeffd0ea0771a94/src/libcrun/linux.c#L1194-L1196
Control then falls through to fallback_to_tmpfs, and cleanup_closep() closes the same descriptor number a second time when the function returns.
The two closes are not adjacent. do_mount() runs in between:
https://github.com/containers/crun/blob/86e7e3eaf8e8d15e6e9983faddeffd0ea0771a94/src/libcrun/linux.c#L1204-L1205
do_mount() opens file descriptors, so the number freed at line 1194 can be reused before the deferred close fires. If the reused descriptor is one that outlives the call (for example stored in the container's private data or an fd map), the cleanup handler closes a live descriptor, and the failure surfaces much later at an unrelated call site.
Suggested fix: add mountfd = -1; after line 1195, or better, stop aliasing an owned descriptor into a cleanup_close variable altogether (see also the sibling issue about the fstat() path).
click me to see the same text rendered as markdown

mount_masked_dir: double close of maskdir_fd when fs_move_mount_to() fails
Title: mount_masked_dir: double close of private_data->maskdir_fd on the move_mount failure path
Body:
Found by code inspection; present on main and at 86e7e3e.
In mount_masked_dir(), mountfd is declared cleanup_close:

cleanup_close int mountfd = -1;

and is then assigned a descriptor that is already owned by private_data:
mountfd = private_data->maskdir_fd;

When fs_move_mount_to() fails, the code closes that descriptor explicitly and clears the struct field, but does not clear mountfd:

crun/src/libcrun/linux.c

Lines 1194 to 1196 in 86e7e3e

TEMP_FAILURE_RETRY (close (private_data->maskdir_fd));
private_data->maskdir_fd = -1;
}

Control then falls through to fallback_to_tmpfs, and cleanup_closep() closes the same descriptor number a second time when the function returns.
The two closes are not adjacent. do_mount() runs in between:

crun/src/libcrun/linux.c

Lines 1204 to 1205 in 86e7e3e

return do_mount (container, "tmpfs", pathfd, rel_path, "tmpfs", MS_RDONLY, "nr_blocks=1,nr_inodes=1",
LABEL_MOUNT | MOUNT_NO_DEFERRED_REMOUNT, err);

do_mount() opens file descriptors, so the number freed at line 1194 can be reused before the deferred close fires. If the reused descriptor is one that outlives the call (for example stored in the container's private data or an fd map), the cleanup handler closes a live descriptor, and the failure surfaces much later at an unrelated call site.
Suggested fix: add mountfd = -1; after line 1195, or better, stop aliasing an owned descriptor into a cleanup_close variable altogether (see also the sibling issue about the fstat() path).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions