* [PATCH] udf: use strscpy() instead of strcpy() for regid ident field
@ 2026-06-27 18:19 Mahad Ibrahim
2026-06-27 21:26 ` David Laight
0 siblings, 1 reply; 3+ messages in thread
From: Mahad Ibrahim @ 2026-06-27 18:19 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-upf, linux-kernel, Mahad Ibrahim
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().
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@gmail.com>
---
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);
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] udf: use strscpy() instead of strcpy() for regid ident field
2026-06-27 18:19 [PATCH] udf: use strscpy() instead of strcpy() for regid ident field Mahad Ibrahim
@ 2026-06-27 21:26 ` David Laight
2026-06-28 11:11 ` Mahad Ibrahim
0 siblings, 1 reply; 3+ messages in thread
From: David Laight @ 2026-06-27 21:26 UTC (permalink / raw)
To: Mahad Ibrahim; +Cc: Jan Kara, linux-upf, linux-kernel
On Sat, 27 Jun 2026 18:19:48 +0000
Mahad Ibrahim <mahad.ibrahim.dev@gmail.com> 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@gmail.com>
> ---
> 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);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] udf: use strscpy() instead of strcpy() for regid ident field
2026-06-27 21:26 ` David Laight
@ 2026-06-28 11:11 ` Mahad Ibrahim
0 siblings, 0 replies; 3+ messages in thread
From: Mahad Ibrahim @ 2026-06-28 11:11 UTC (permalink / raw)
To: David Laight; +Cc: Jan Kara, linux-upf, linux-kernel
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@gmail.com> wrote:
>
> On Sat, 27 Jun 2026 18:19:48 +0000
> Mahad Ibrahim <mahad.ibrahim.dev@gmail.com> 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@gmail.com>
> > ---
> > 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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-28 11:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-27 18:19 [PATCH] udf: use strscpy() instead of strcpy() for regid ident field Mahad Ibrahim
2026-06-27 21:26 ` David Laight
2026-06-28 11:11 ` Mahad Ibrahim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.