On Tue, Jun 17, 2025 at 06:15:46PM +0530, Pranav Tyagi wrote: > Replace the deprecated strncpy() with strscpy() as the destination > buffer should be NUL-terminated and does not require any trailing > NUL-padding. Also, since NUL-termination is guaranteed, NUL-termination is only guaranteed if you copy into the buffer one less byte than the label requires, i.e XFSLABEL_MAX. > use sizeof(label) in place of XFSLABEL_MAX as the size > parameter. This is wrong, see below why. > > Signed-off-by: Pranav Tyagi <pranav.tyagi03@xxxxxxxxx> > --- > fs/xfs/xfs_ioctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index d250f7f74e3b..9f4d68c5b5ab 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -992,7 +992,7 @@ xfs_ioc_getlabel( > /* 1 larger than sb_fname, so this ensures a trailing NUL char */ > memset(label, 0, sizeof(label)); > spin_lock(&mp->m_sb_lock); > - strncpy(label, sbp->sb_fname, XFSLABEL_MAX); > + strscpy(label, sbp->sb_fname, sizeof(label)); This is broken and you created a buffer overrun here. XFSLABEL_MAX is set to 12 bytes. The current label size is 13 bytes: char label[XFSLABEL_MAX + 1]; This ensures the label will always have a null termination character as long as you copy XFSLABEL_MAX bytes into the label. - strncpy(label, sbp->sb_fname, XFSLABEL_MAX); Copies 12 bytes from sb_fname into label. This ensures we always have a trailing \0 at the last byte. Your version: strscpy(label, sbp->sb_fname, sizeof(label)); Copies 13 bytes from sb_fname into the label buffer. This not only could have copied a non-null byte to the last byte in the label buffer, but also But sbp->sb_fname size is XFSLABEL_MAX, so you are reading beyond the source buffer size, causing a buffer overrun as you can see on the kernel test robot report. Carlos > spin_unlock(&mp->m_sb_lock); > > if (copy_to_user(user_label, label, sizeof(label))) > -- > 2.49.0 >
