Em qui., 17 de jul. de 2025 às 13:39, Darrick J. Wong <djwong@xxxxxxxxxx> escreveu: > > On Wed, Jul 16, 2025 at 03:20:37PM -0300, Marcelo Moreira wrote: > > The `strncpy` function is deprecated for NUL-terminated strings as > > explained in the "strncpy() on NUL-terminated strings" section of > > Documentation/process/deprecated.rst. > > > > In `xrep_symlink_salvage_inline()`, the `target_buf` (which is `sc->buf`) > > is intended to hold a NUL-terminated symlink path. The original code > > used `strncpy(target_buf, ifp->if_data, nr)`, where `nr` is the maximum > > number of bytes to copy. This approach is problematic because `strncpy()` > > does not guarantee NUL-termination if the source string is truncated > > exactly at `nr` bytes, which can lead to out-of-bounds read issues > > if the buffer is later treated as a NUL-terminated string. > > Evidence from `fs/xfs/scrub/symlink.c` (e.g., `strnlen(sc->buf, > > XFS_SYMLINK_MAXLEN)`) confirms that `sc->buf` is indeed expected to be > > NUL-terminated. Furthermore, `sc->buf` is allocated with > > `kvzalloc(XFS_SYMLINK_MAXLEN + 1, ...)`, explicitly reserving space for > > the NUL terminator. > > > > `strscpy()` is the proper replacement because it guarantees NUL-termination > > of the destination buffer, correctly handles the copy limit, and aligns > > with current kernel string-copying best practices. > > Other recommended functions like `strscpy_pad()`, `memcpy()`, or > > `memcpy_and_pad()` were not used because: > > - `strscpy_pad()` would unnecessarily zero-pad the entire buffer beyond the > > NUL terminator, which is not required as the function returns `nr` bytes. > > - `memcpy()` and `memcpy_and_pad()` do not guarantee NUL-termination, which > > is critical given `target_buf` is used as a NUL-terminated string. > > > > This change improves code safety and clarity by using a safer function for > > string copying. > > Did you find an actual bug in online fsck, or is this just > s/strncpy/strscpy/ ? If the code already works correctly, please leave > it alone. Unless you want to take on all the online fsck and fuzz > testing to make sure the changes don't break anything. This was s/strncpy/strscpy/ , no real bug. -- Cheers, Marcelo Moreira
