On Tue, Jun 24, 2025 at 04:15:37PM +0200, Jan Kara wrote: > On Tue 24-06-25 12:59:26, Christian Brauner wrote: > > > For pidfs this means a file handle can function as full replacement for > > > storing a pid in a file. Instead a file handle can be stored and > > > reopened purely based on the file handle. > > > > One thing I want to comment on generally. I know we document that a file > > handle is an opaque thing and userspace shouldn't rely on the layout or > > format (Propably so that we're free to redefine it.). > > > > Realistically though that's just not what's happening. I've linked Amir > > to that code already a few times but I'm doing it here for all of you > > again: > > > > [1]: https://github.com/systemd/systemd/blob/7e1647ae4e33dd8354bd311a7f7f5eb701be2391/src/basic/cgroup-util.c#L62-L77 > > > > Specifically: > > > > /* The structure to pass to name_to_handle_at() on cgroupfs2 */ > > typedef union { > > struct file_handle file_handle; > > uint8_t space[offsetof(struct file_handle, f_handle) + sizeof(uint64_t)]; > > } cg_file_handle; > > > > #define CG_FILE_HANDLE_INIT \ > > (cg_file_handle) { \ > > .file_handle.handle_bytes = sizeof(uint64_t), \ > > .file_handle.handle_type = FILEID_KERNFS, \ > > } > > > > #define CG_FILE_HANDLE_CGROUPID(fh) (*CAST_ALIGN_PTR(uint64_t, (fh).file_handle.f_handle)) > > > > cg_file_handle fh = CG_FILE_HANDLE_INIT; > > CG_FILE_HANDLE_CGROUPID(fh) = id; > > > > return RET_NERRNO(open_by_handle_at(cgroupfs_fd, &fh.file_handle, O_DIRECTORY|O_CLOEXEC)); > > > > Another example where the layout is assumed to be uapi/uabi is: > > > > [2]: https://github.com/systemd/systemd/blob/7e1647ae4e33dd8354bd311a7f7f5eb701be2391/src/basic/pidfd-util.c#L232-L259 > > > > int pidfd_get_inode_id_impl(int fd, uint64_t *ret) { > > <snip> > > union { > > struct file_handle file_handle; > > uint8_t space[offsetof(struct file_handle, f_handle) + sizeof(uint64_t)]; > > } fh = { > > .file_handle.handle_bytes = sizeof(uint64_t), > > .file_handle.handle_type = FILEID_KERNFS, > > }; > > int mnt_id; > > > > r = RET_NERRNO(name_to_handle_at(fd, "", &fh.file_handle, &mnt_id, AT_EMPTY_PATH)); > > if (r >= 0) { > > if (ret) > > *ret = *CAST_ALIGN_PTR(uint64_t, fh.file_handle.f_handle); > > return 0; > > } > > Thanks for sharing. Sigh... Personal note for the future: If something > should be opaque blob for userspace, don't forget to encrypt the data > before handing it over to userspace. :-P Yeah, honestly, that's what we should probably do. Use some hash function or something.