Re: [PATCH] udf: use strscpy() instead of strcpy() for regid ident field

[Mahad Ibrahim]

Makes sense, thanks.

The destination is a fixed array and the source is
a constant, so strcpy() is already safe here and fails louder on
overflow. Dropping the patch.


On Sun, Jun 28, 2026 at 2:26 AM David Laight
<david.laight.linux@xxxxxxxxx> wrote:
>
> On Sat, 27 Jun 2026 18:19:48 +0000
> Mahad Ibrahim <mahad.ibrahim.dev@xxxxxxxxx> wrote:
>
> > strcpy() is deprecated as it performs no bounds checking. Replace the
> > three call sites that copy UDF_ID_DEVELOPER into the regid ident field
> > with strscpy().
>
> There is no real reason to disallow use of strcpy() to copy constant
> strings into arrays.
> The compiler (or rather the header files) can allow such safe uses
> while rejecting ones that might potentially overflow.
>
> Additionally if the fixed string is too long the compiler will generate
> an error for strcpy() whereas strscpy() will truncate the copy.
>
> So this change really is pointless churn.
>
>         David
>
> >
> > The current string fits the field with room to spare, so there is no
> > overflow today. strscpy() bounds the copy to the destination and
> > NUL-terminates, keeping it safe if the string or the field size
> > changes later.
> >
> > Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@xxxxxxxxx>
> > ---
> >  fs/udf/inode.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> > index 67bcf83758c8..3140e001b315 100644
> > --- a/fs/udf/inode.c
> > +++ b/fs/udf/inode.c
> > @@ -1809,7 +1809,7 @@ static int udf_update_inode(struct inode *inode, int do_sync)
> >               }
> >               eid = (struct regid *)dsea->impUse;
> >               memset(eid, 0, sizeof(*eid));
> > -             strcpy(eid->ident, UDF_ID_DEVELOPER);
> > +             strscpy(eid->ident, UDF_ID_DEVELOPER, sizeof(eid->ident));
> >               eid->identSuffix[0] = UDF_OS_CLASS_UNIX;
> >               eid->identSuffix[1] = UDF_OS_ID_LINUX;
> >               dsea->majorDeviceIdent = cpu_to_le32(imajor(inode));
> > @@ -1833,7 +1833,7 @@ static int udf_update_inode(struct inode *inode, int do_sync)
> >               udf_time_to_disk_stamp(&fe->modificationTime, inode_get_mtime(inode));
> >               udf_time_to_disk_stamp(&fe->attrTime, inode_get_ctime(inode));
> >               memset(&(fe->impIdent), 0, sizeof(struct regid));
> > -             strcpy(fe->impIdent.ident, UDF_ID_DEVELOPER);
> > +             strscpy(fe->impIdent.ident, UDF_ID_DEVELOPER, sizeof(fe->impIdent.ident));
> >               fe->impIdent.identSuffix[0] = UDF_OS_CLASS_UNIX;
> >               fe->impIdent.identSuffix[1] = UDF_OS_ID_LINUX;
> >               fe->uniqueID = cpu_to_le64(iinfo->i_unique);
> > @@ -1872,7 +1872,7 @@ static int udf_update_inode(struct inode *inode, int do_sync)
> >               udf_time_to_disk_stamp(&efe->attrTime, inode_get_ctime(inode));
> >
> >               memset(&(efe->impIdent), 0, sizeof(efe->impIdent));
> > -             strcpy(efe->impIdent.ident, UDF_ID_DEVELOPER);
> > +             strscpy(efe->impIdent.ident, UDF_ID_DEVELOPER, sizeof(efe->impIdent.ident));
> >               efe->impIdent.identSuffix[0] = UDF_OS_CLASS_UNIX;
> >               efe->impIdent.identSuffix[1] = UDF_OS_ID_LINUX;
> >               efe->uniqueID = cpu_to_le64(iinfo->i_unique);
>