On Wed, Jan 08, 2025 at 02:43:12PM +0100, Alice Ryhl wrote: > On Tue, Jan 7, 2025 at 7:48 PM Isaac J. Manjarres > <isaacmanjarres@xxxxxxxxxx> wrote: > > > > The existing logic uses strnlen_user() to calculate the length of the > > memfd name from userspace and then copies the string into a buffer using > > copy_from_user(). This is error-prone, as the string length > > could have changed between the time when it was calculated and when the > > string was copied. The existing logic handles this by ensuring that the > > last byte in the buffer is the terminating zero. > > > > This handling is contrived and can better be handled by using > > strncpy_from_user(), which gets the length of the string and copies > > it in one shot. Therefore, simplify the logic for copying the memfd > > name by using strncpy_from_user(). > > > > No functional change. > > > > Signed-off-by: Isaac J. Manjarres <isaacmanjarres@xxxxxxxxxx> > > Looks okay to me. One nit below, but: > > Reviewed-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> > > > + /* length does not include terminating zero */ > > + len = strncpy_from_user(name + MFD_NAME_PREFIX_LEN, uname, MFD_NAME_MAX_LEN + 1); > > Can we have this comment say "returned length" instead of just > "length"? Or just remove the comment. Initially I thought you were > talking about the last argument, and I was confused as that does > include the nul-terminator. > > Alice Yes, I will update it to say returned length and add your "Reviewed-by" tag. Thanks for this! --Isaac
