On Mon, Oct 19, 2020 at 12:26:53PM +0200, Giuseppe Scrivano wrote: > When the flag CLOSE_RANGE_CLOEXEC is set, close_range doesn't > immediately close the files but it sets the close-on-exec bit. > > It is useful for e.g. container runtimes that usually install a > seccomp profile "as late as possible" before execv'ing the container > process itself. The container runtime could either do: > 1 2 > - install_seccomp_profile(); - close_range(MIN_FD, MAX_INT, 0); > - close_range(MIN_FD, MAX_INT, 0); - install_seccomp_profile(); > - execve(...); - execve(...); > > Both alternative have some disadvantages. > > In the first variant the seccomp_profile cannot block the close_range > syscall, as well as opendir/read/close/... for the fallback on older > kernels). > In the second variant, close_range() can be used only on the fds > that are not going to be needed by the runtime anymore, and it must be > potentially called multiple times to account for the different ranges > that must be closed. > > Using close_range(..., ..., CLOSE_RANGE_CLOEXEC) solves these issues. > The runtime is able to use the open fds and the seccomp profile could > block close_range() and the syscalls used for its fallback.
I see, so you want those fds to be closed after exec but still use them before. Yeah, this is a good use-case. (I proposed this extension quite a while ago when we started discussing this syscall. Thanks for working ont this!) > > Signed-off-by: Giuseppe Scrivano <[email protected]> > --- > fs/file.c | 44 ++++++++++++++++++++++++-------- > include/uapi/linux/close_range.h | 3 +++ > 2 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/fs/file.c b/fs/file.c > index 21c0893f2f1d..0295d4f7c5ef 100644 > --- a/fs/file.c > +++ b/fs/file.c > @@ -672,6 +672,35 @@ int __close_fd(struct files_struct *files, unsigned fd) > } > EXPORT_SYMBOL(__close_fd); /* for ksys_close() */ > > +static inline void __range_cloexec(struct files_struct *cur_fds, > + unsigned int fd, unsigned int max_fd) > +{ > + struct fdtable *fdt; > + > + if (fd > max_fd) > + return; Looks like formatting issues here. > + > + spin_lock(&cur_fds->file_lock); > + fdt = files_fdtable(cur_fds); > + bitmap_set(fdt->close_on_exec, fd, max_fd - fd + 1); I think that this is ok and that there's no reason to make this anymore complex unless we somehow really see performance issues which I doubt. If Al is ok with doing it this way and doesn't see any obvious issues I'll be taking this for some testing and would come back to ack this and pick it up. > + spin_unlock(&cur_fds->file_lock); > +} > + > +static inline void __range_close(struct files_struct *cur_fds, unsigned int > fd, > + unsigned int max_fd) > +{ > + while (fd <= max_fd) { > + struct file *file; > + > + file = pick_file(cur_fds, fd++); > + if (!file) > + continue; > + > + filp_close(file, cur_fds); > + cond_resched(); > + } > +} > + > /** > * __close_range() - Close all file descriptors in a given range. > * > @@ -687,7 +716,7 @@ int __close_range(unsigned fd, unsigned max_fd, unsigned > int flags) > struct task_struct *me = current; > struct files_struct *cur_fds = me->files, *fds = NULL; > > - if (flags & ~CLOSE_RANGE_UNSHARE) > + if (flags & ~(CLOSE_RANGE_UNSHARE | CLOSE_RANGE_CLOEXEC)) > return -EINVAL; > > if (fd > max_fd) > @@ -725,16 +754,11 @@ int __close_range(unsigned fd, unsigned max_fd, > unsigned int flags) > } > > max_fd = min(max_fd, cur_max); > - while (fd <= max_fd) { > - struct file *file; > > - file = pick_file(cur_fds, fd++); > - if (!file) > - continue; > - > - filp_close(file, cur_fds); > - cond_resched(); > - } > + if (flags & CLOSE_RANGE_CLOEXEC) > + __range_cloexec(cur_fds, fd, max_fd); > + else > + __range_close(cur_fds, fd, max_fd); > > if (fds) { > /* > diff --git a/include/uapi/linux/close_range.h > b/include/uapi/linux/close_range.h > index 6928a9fdee3c..2d804281554c 100644 > --- a/include/uapi/linux/close_range.h > +++ b/include/uapi/linux/close_range.h > @@ -5,5 +5,8 @@ > /* Unshare the file descriptor table before closing file descriptors. */ > #define CLOSE_RANGE_UNSHARE (1U << 1) > > +/* Set the FD_CLOEXEC bit instead of closing the file descriptor. */ > +#define CLOSE_RANGE_CLOEXEC (1U << 2) > + > #endif /* _UAPI_LINUX_CLOSE_RANGE_H */ > > -- > 2.26.2 > > _______________________________________________ > Containers mailing list > [email protected] > https://lists.linuxfoundation.org/mailman/listinfo/containers
