-
Notifications
You must be signed in to change notification settings - Fork 3.2k
uuidgen: generate UUIDs in bounded batches to respect kernel limit #1965
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you for taking the time to contribute to FreeBSD! All issues resolved. |
b406ddf to
6e48b12
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6e48b12 to
a00e249
Compare
|
Hello, All the CI checks are completed (or in progress for internal CI). Just double-checking if there’s anything else that I should adjust or work on regarding this change. Thanks for your time. |
ac95ee2 to
47696d8
Compare
bsdimp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2048 being hard coded seems weird, but I don't see a #define or sysctl to get this value.
The uuidgen(2) system call enforces a hard upper limit of 2048 UUIDs per invocation. uuidgen(1) previously attempted to generate arbitrary counts in a single call and allocated memory accordingly, leading to EINVAL errors, unnecessary memory usage, and potential overflow risks. Generate UUIDs in fixed-size batches, streaming output incrementally while preserving existing semantics. Mirror the kernel limit explicitly since it is not exposed via a public interface. Signed-off-by: NVSRahul <[email protected]>
47696d8 to
9376f5f
Compare
|
Thanks for the review! I agree the hard-coded limit looks odd. Since the uuidgen(2) limit is enforced in the kernel and not exposed via a sysctl or public header, I’ve renamed the constant to UUIDGEN_BATCH_MAX and added a comment pointing directly to sys/kern/kern_uuid.c to document the origin and rationale. Please let me know if you’d prefer this handled differently. |
Fix uuidgen(1) to generate UUIDs in bounded batches that respect the
kernel uuidgen(2) limit.
The kernel uuidgen(2) system call is limited to 2048 UUIDs per call, but
uuidgen(1) attempted to generate arbitrarily many UUIDs in a single
invocation and allocate memory proportional to the request size. This
can lead to kernel errors, unnecessary memory usage, and potential
size_t overflow.
This change generates UUIDs in fixed-size batches, preserving streaming
semantics while respecting the kernel ABI and avoiding unbounded memory
allocation.