[PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90

public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed

* [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90
@ 2025-04-02 17:25 goralbaris
  2025-04-02 19:14 ` Greg KH
  2025-04-02 20:11 ` [[PATCH v2]] transform strncpy into strscpy Baris Can Goral
  0 siblings, 2 replies; 13+ messages in thread
From: goralbaris @ 2025-04-02 17:25 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees, goralbaris

Signed-off-by: goralbaris <goralbaris@gmail.com>
---
 drivers/target/target_core_configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, read_bytes);
+	strscpy(db_root, db_root_stage, read_bytes);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 
 	r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90
  2025-04-02 17:25 [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90 goralbaris
@ 2025-04-02 19:14 ` Greg KH
  2025-04-02 20:11 ` [[PATCH v2]] transform strncpy into strscpy Baris Can Goral
  1 sibling, 0 replies; 13+ messages in thread
From: Greg KH @ 2025-04-02 19:14 UTC (permalink / raw)
  To: goralbaris
  Cc: martin.petersen, linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

On Wed, Apr 02, 2025 at 08:25:04PM +0300, goralbaris wrote:
> Signed-off-by: goralbaris <goralbaris@gmail.com>

I think something went wrong with your subject line :(

Also, don't use your email alias for signed-off-by please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [[PATCH v2]] transform strncpy into strscpy
  2025-04-02 17:25 [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90 goralbaris
  2025-04-02 19:14 ` Greg KH
@ 2025-04-02 20:11 ` Baris Can Goral
  2025-04-02 20:45   ` [PATCH v3] " Baris Can Goral
  1 sibling, 1 reply; 13+ messages in thread
From: Baris Can Goral @ 2025-04-02 20:11 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees, Baris Can Goral

Description:
The strncpy() function is actively dangerous to use since it may not NULL-terminate the destination string,
resulting in potential memory content exposures, unbounded reads, or crashes.
Link: [1] https://github.com/KSPP/linux/issues/90
Changes from v2:
	-Description added
	-User Name corrected
Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
 drivers/target/target_core_configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, read_bytes);
+	strscpy(db_root, db_root_stage, read_bytes);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 
 	r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3] transform strncpy into strscpy
  2025-04-02 20:11 ` [[PATCH v2]] transform strncpy into strscpy Baris Can Goral
@ 2025-04-02 20:45   ` Baris Can Goral
  2025-04-03 12:41     ` Maurizio Lombardi
  2025-04-05 14:36     ` [PATCH v4] scsi: target: " Baris Can Goral
  0 siblings, 2 replies; 13+ messages in thread
From: Baris Can Goral @ 2025-04-02 20:45 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees, Baris Can Goral

Description:
The strncpy() function is actively dangerous to use since it may not 
NULL-terminate the destination string,resulting in potential memory 
content exposures, unbounded reads, or crashes.

Link:https://github.com/KSPP/linux/issues/90
Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
Changes from v3:
	-Description added
	-User name corrected
	-formatting issues.
 drivers/target/target_core_configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, read_bytes);
+	strscpy(db_root, db_root_stage, read_bytes);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 
 	r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] transform strncpy into strscpy
  2025-04-02 20:45   ` [PATCH v3] " Baris Can Goral
@ 2025-04-03 12:41     ` Maurizio Lombardi
  2025-04-05 14:36     ` [PATCH v4] scsi: target: " Baris Can Goral
  1 sibling, 0 replies; 13+ messages in thread
From: Maurizio Lombardi @ 2025-04-03 12:41 UTC (permalink / raw)
  To: Baris Can Goral, martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

Hello, two small things:

On Wed Apr 2, 2025 at 10:45 PM CEST, Baris Can Goral wrote:
> Description:

You can remove this "description" tag.
Also, it's better to add a prefix to the subject, for example:

"scsi: target: transform strncpy into strscpy"

Maurizio

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4] scsi: target: transform strncpy into strscpy
  2025-04-02 20:45   ` [PATCH v3] " Baris Can Goral
  2025-04-03 12:41     ` Maurizio Lombardi
@ 2025-04-05 14:36     ` Baris Can Goral
  2025-04-05 15:25       ` David Laight
  2025-04-07  7:53       ` Maurizio Lombardi
  1 sibling, 2 replies; 13+ messages in thread
From: Baris Can Goral @ 2025-04-05 14:36 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees, Baris Can Goral

The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string,resulting in potential memory
content exposures, unbounded reads, or crashes.

Link:https://github.com/KSPP/linux/issues/90
Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
Changes from v4:
	-Description added
	-User name corrected
	-formatting issues.
	-commit name changed
 drivers/target/target_core_configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, read_bytes);
+	strscpy(db_root, db_root_stage, read_bytes);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 
 	r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4] scsi: target: transform strncpy into strscpy
  2025-04-05 14:36     ` [PATCH v4] scsi: target: " Baris Can Goral
@ 2025-04-05 15:25       ` David Laight
       [not found]         ` <CAJOJxizEDm_th4G=BvejM4_jGcF6+QYT=LjD_J_FTbsNFVTjCQ@mail.gmail.com>
  2025-04-07  7:53       ` Maurizio Lombardi
  1 sibling, 1 reply; 13+ messages in thread
From: David Laight @ 2025-04-05 15:25 UTC (permalink / raw)
  To: Baris Can Goral
  Cc: martin.petersen, linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

On Sat,  5 Apr 2025 17:36:47 +0300
Baris Can Goral <goralbaris@gmail.com> wrote:

> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string,resulting in potential memory
> content exposures, unbounded reads, or crashes.
> 
> Link:https://github.com/KSPP/linux/issues/90
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
> Changes from v4:
> 	-Description added
> 	-User name corrected
> 	-formatting issues.
> 	-commit name changed
>  drivers/target/target_core_configfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index c40217f44b1b..5c0b74e76be2 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
>  	}
>  	filp_close(fp, NULL);
>  
> -	strncpy(db_root, db_root_stage, read_bytes);
> +	strscpy(db_root, db_root_stage, read_bytes);
>  	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);

That code is broken, it reads:
	read_bytes = snprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
	if (!read_bytes)
		goto unlock;

	if (db_root_stage[read_bytes - 1] == '\n')
		db_root_stage[read_bytes - 1] = '\0';

	/* validate new db root before accepting it */
	fp = filp_open(db_root_stage, O_RDONLY, 0);
	if (IS_ERR(fp)) {
		pr_err("db_root: cannot open: %s\n", db_root_stage);
		goto unlock;
	}
	if (!S_ISDIR(file_inode(fp)->i_mode)) {
		filp_close(fp, NULL);
		pr_err("db_root: not a directory: %s\n", db_root_stage);
		goto unlock;
	}
	filp_close(fp, NULL);

	strncpy(db_root, db_root_stage, read_bytes);
	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);

	r = read_bytes;

unlock:
	mutex_unlock(&target_devices_lock);
	return r;

'Really nasty (tm)' things happen if 'page' is too long.

	David

>  
>  	r = read_bytes;
> @@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
>  	}
>  	filp_close(fp, NULL);
>  
> -	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
> +	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
>  	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>  }
>  


^ permalink raw reply	[flat|nested] 13+ messages in thread

[parent not found: <CAJOJxizEDm_th4G=BvejM4_jGcF6+QYT=LjD_J_FTbsNFVTjCQ@mail.gmail.com>]

* Re: [PATCH v4] scsi: target: transform strncpy into strscpy
       [not found]         ` <CAJOJxizEDm_th4G=BvejM4_jGcF6+QYT=LjD_J_FTbsNFVTjCQ@mail.gmail.com>
@ 2025-04-05 17:28           ` baris goral
  2025-04-05 20:07           ` David Laight
  1 sibling, 0 replies; 13+ messages in thread
From: baris goral @ 2025-04-05 17:28 UTC (permalink / raw)
  To: David Laight
  Cc: martin.petersen, linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

Hi,
Trying to understand, it has if check a few lines above:

if (count > (DB_ROOT_LEN - 1))

Does not it met our expectations?


Best Reagrds,
Baris


baris goral <goralbaris@gmail.com>, 5 Nis 2025 Cmt, 19:35 tarihinde şunu yazdı:
>
> Hi,
> Trying to understand, it has if check a few lines above:
>
> if (count > (DB_ROOT_LEN - 1))
>
> Does not it met our expectations?
>
>
> David Laight <david.laight.linux@gmail.com>, 5 Nis 2025 Cmt, 18:25 tarihinde şunu yazdı:
>>
>> On Sat,  5 Apr 2025 17:36:47 +0300
>> Baris Can Goral <goralbaris@gmail.com> wrote:
>>
>> > The strncpy() function is actively dangerous to use since it may not
>> > NULL-terminate the destination string,resulting in potential memory
>> > content exposures, unbounded reads, or crashes.
>> >
>> > Link:https://github.com/KSPP/linux/issues/90
>> > Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
>> > ---
>> > Changes from v4:
>> >       -Description added
>> >       -User name corrected
>> >       -formatting issues.
>> >       -commit name changed
>> >  drivers/target/target_core_configfs.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
>> > index c40217f44b1b..5c0b74e76be2 100644
>> > --- a/drivers/target/target_core_configfs.c
>> > +++ b/drivers/target/target_core_configfs.c
>> > @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
>> >       }
>> >       filp_close(fp, NULL);
>> >
>> > -     strncpy(db_root, db_root_stage, read_bytes);
>> > +     strscpy(db_root, db_root_stage, read_bytes);
>> >       pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>>
>> That code is broken, it reads:
>>         read_bytes = snprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
>>         if (!read_bytes)
>>                 goto unlock;
>>
>>         if (db_root_stage[read_bytes - 1] == '\n')
>>                 db_root_stage[read_bytes - 1] = '\0';
>>
>>         /* validate new db root before accepting it */
>>         fp = filp_open(db_root_stage, O_RDONLY, 0);
>>         if (IS_ERR(fp)) {
>>                 pr_err("db_root: cannot open: %s\n", db_root_stage);
>>                 goto unlock;
>>         }
>>         if (!S_ISDIR(file_inode(fp)->i_mode)) {
>>                 filp_close(fp, NULL);
>>                 pr_err("db_root: not a directory: %s\n", db_root_stage);
>>                 goto unlock;
>>         }
>>         filp_close(fp, NULL);
>>
>>         strncpy(db_root, db_root_stage, read_bytes);
>>         pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>>
>>         r = read_bytes;
>>
>> unlock:
>>         mutex_unlock(&target_devices_lock);
>>         return r;
>>
>> 'Really nasty (tm)' things happen if 'page' is too long.
>>
>>         David
>>
>> >
>> >       r = read_bytes;
>> > @@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
>> >       }
>> >       filp_close(fp, NULL);
>> >
>> > -     strncpy(db_root, db_root_stage, DB_ROOT_LEN);
>> > +     strscpy(db_root, db_root_stage, DB_ROOT_LEN);
>> >       pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>> >  }
>> >
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4] scsi: target: transform strncpy into strscpy
       [not found]         ` <CAJOJxizEDm_th4G=BvejM4_jGcF6+QYT=LjD_J_FTbsNFVTjCQ@mail.gmail.com>
  2025-04-05 17:28           ` baris goral
@ 2025-04-05 20:07           ` David Laight
  1 sibling, 0 replies; 13+ messages in thread
From: David Laight @ 2025-04-05 20:07 UTC (permalink / raw)
  To: baris goral
  Cc: martin.petersen, linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

On Sat, 5 Apr 2025 19:35:01 +0300
baris goral <goralbaris@gmail.com> wrote:

> Hi,
> Trying to understand, it has if check a few lines above:
> 
> if (count > (DB_ROOT_LEN - 1))
> 
> Does not it met our expectations?

Don't top post on mailing lists.

The first issue is that the return value of snprintf() is the number
of characters that would be written into the buffer were it long enough.
The kernel's scnprintf() will return the number of characters written.

But why is it using snprintf() just to copy a string?
Why is truncation at all safe here?
Why is a '\n' being removed without the length being changed.
The length argument to strscpy() should be the length of the destination
(to stop overruns), not the number of characters.
In this case it is the number of characters - so will delete another
character (unless a '\n' was removed).
The return value is just garbage.

You may have opened a bag of worms, but you've also made it worse.

	David

> 
> David Laight <david.laight.linux@gmail.com>, 5 Nis 2025 Cmt, 18:25
> tarihinde şunu yazdı:
> 
> > On Sat,  5 Apr 2025 17:36:47 +0300
> > Baris Can Goral <goralbaris@gmail.com> wrote:
> >  
> > > The strncpy() function is actively dangerous to use since it may not
> > > NULL-terminate the destination string,resulting in potential memory
> > > content exposures, unbounded reads, or crashes.
> > >
> > > Link:https://github.com/KSPP/linux/issues/90
> > > Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> > > ---
> > > Changes from v4:
> > >       -Description added
> > >       -User name corrected
> > >       -formatting issues.
> > >       -commit name changed
> > >  drivers/target/target_core_configfs.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/target/target_core_configfs.c  
> > b/drivers/target/target_core_configfs.c  
> > > index c40217f44b1b..5c0b74e76be2 100644
> > > --- a/drivers/target/target_core_configfs.c
> > > +++ b/drivers/target/target_core_configfs.c
> > > @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct  
> > config_item *item,  
> > >       }
> > >       filp_close(fp, NULL);
> > >
> > > -     strncpy(db_root, db_root_stage, read_bytes);
> > > +     strscpy(db_root, db_root_stage, read_bytes);
> > >       pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);  
> >
> > That code is broken, it reads:
> >         read_bytes = snprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
> >         if (!read_bytes)
> >                 goto unlock;
> >
> >         if (db_root_stage[read_bytes - 1] == '\n')
> >                 db_root_stage[read_bytes - 1] = '\0';
> >
> >         /* validate new db root before accepting it */
> >         fp = filp_open(db_root_stage, O_RDONLY, 0);
> >         if (IS_ERR(fp)) {
> >                 pr_err("db_root: cannot open: %s\n", db_root_stage);
> >                 goto unlock;
> >         }
> >         if (!S_ISDIR(file_inode(fp)->i_mode)) {
> >                 filp_close(fp, NULL);
> >                 pr_err("db_root: not a directory: %s\n", db_root_stage);
> >                 goto unlock;
> >         }
> >         filp_close(fp, NULL);
> >
> >         strncpy(db_root, db_root_stage, read_bytes);
> >         pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> >
> >         r = read_bytes;
> >
> > unlock:
> >         mutex_unlock(&target_devices_lock);
> >         return r;
> >
> > 'Really nasty (tm)' things happen if 'page' is too long.
> >
> >         David
> >  
> > >
> > >       r = read_bytes;
> > > @@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
> > >       }
> > >       filp_close(fp, NULL);
> > >
> > > -     strncpy(db_root, db_root_stage, DB_ROOT_LEN);
> > > +     strscpy(db_root, db_root_stage, DB_ROOT_LEN);
> > >       pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> > >  }
> > >  
> >
> >  


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4] scsi: target: transform strncpy into strscpy
  2025-04-05 14:36     ` [PATCH v4] scsi: target: " Baris Can Goral
  2025-04-05 15:25       ` David Laight
@ 2025-04-07  7:53       ` Maurizio Lombardi
  2025-04-07 17:48         ` Baris Can Goral
  1 sibling, 1 reply; 13+ messages in thread
From: Maurizio Lombardi @ 2025-04-07  7:53 UTC (permalink / raw)
  To: Baris Can Goral, martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

On Sat Apr 5, 2025 at 4:36 PM CEST, Baris Can Goral wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string,resulting in potential memory
> content exposures, unbounded reads, or crashes.
>
> Link:https://github.com/KSPP/linux/issues/90
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
> Changes from v4:
> 	-Description added
> 	-User name corrected
> 	-formatting issues.
> 	-commit name changed
>  drivers/target/target_core_configfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index c40217f44b1b..5c0b74e76be2 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
>  	}
>  	filp_close(fp, NULL);
>  
> -	strncpy(db_root, db_root_stage, read_bytes);
> +	strscpy(db_root, db_root_stage, read_bytes);
>  	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>  
>  	r = read_bytes;
> @@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
>  	}
>  	filp_close(fp, NULL);
>  
> -	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
> +	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
>  	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>  }
>  

This patch doesn't apply anymore.
strncpy() has already been replaced with strscpy()
in version 6.14-rc2.


Maurizio

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4] scsi: target: transform strncpy into strscpy
  2025-04-07  7:53       ` Maurizio Lombardi
@ 2025-04-07 17:48         ` Baris Can Goral
  2025-04-08  7:18           ` Maurizio Lombardi
  0 siblings, 1 reply; 13+ messages in thread
From: Baris Can Goral @ 2025-04-07 17:48 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

Hi Maurizio,

Unfourtunately, in version 6.14-rc2 (and also v6.14) strncpy is still there.

Best Regards,
Baris

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4] scsi: target: transform strncpy into strscpy
  2025-04-07 17:48         ` Baris Can Goral
@ 2025-04-08  7:18           ` Maurizio Lombardi
  0 siblings, 0 replies; 13+ messages in thread
From: Maurizio Lombardi @ 2025-04-08  7:18 UTC (permalink / raw)
  To: Baris Can Goral, martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees

On Mon Apr 7, 2025 at 7:48 PM CEST, Baris Can Goral wrote:
> Hi Maurizio,
>
> Unfourtunately, in version 6.14-rc2 (and also v6.14) strncpy is still there.

Sorry, my mistake.

commit dfb7df1ddb29c89662e84b2c82c1ff7943358ae0 that replaces
strncpy() with strscpy() has been merged in kernel version 6.15-rc1

Maurizio

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90
@ 2025-04-02 17:33 goralbaris
  0 siblings, 0 replies; 13+ messages in thread
From: goralbaris @ 2025-04-02 17:33 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, target-devel, linux-kernel, skhan,
	linux-kernel-mentees, goralbaris

The strncpy is depricated. 
It may not NUL-terminate the destination string, resulting in potential memory
content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90
Signed-off-by: goralbaris <goralbaris@gmail.com>
---
 drivers/target/target_core_configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, read_bytes);
+	strscpy(db_root, db_root_stage, read_bytes);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 
 	r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+	strscpy(db_root, db_root_stage, DB_ROOT_LEN);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-04-08  7:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-02 17:25 [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90 goralbaris
2025-04-02 19:14 ` Greg KH
2025-04-02 20:11 ` [[PATCH v2]] transform strncpy into strscpy Baris Can Goral
2025-04-02 20:45   ` [PATCH v3] " Baris Can Goral
2025-04-03 12:41     ` Maurizio Lombardi
2025-04-05 14:36     ` [PATCH v4] scsi: target: " Baris Can Goral
2025-04-05 15:25       ` David Laight
     [not found]         ` <CAJOJxizEDm_th4G=BvejM4_jGcF6+QYT=LjD_J_FTbsNFVTjCQ@mail.gmail.com>
2025-04-05 17:28           ` baris goral
2025-04-05 20:07           ` David Laight
2025-04-07  7:53       ` Maurizio Lombardi
2025-04-07 17:48         ` Baris Can Goral
2025-04-08  7:18           ` Maurizio Lombardi
  -- strict thread matches above, loose matches on Subject: below --
2025-04-02 17:33 [PATCH] change strncpy to strscpy strncpy is now depricated. It may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Link: https://github.com/KSPP/linux/issues/90 goralbaris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox