diff mbox series

[f2fs-dev,v2] f2fs: replace deprecated strcpy with strscpy

Message ID 20241109013819.5952-1-danielyangkang@gmail.com (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev,v2] f2fs: replace deprecated strcpy with strscpy | expand

Commit Message

Daniel Yang Nov. 9, 2024, 1:38 a.m. UTC
strcpy is deprecated. Kernel docs recommend replacing strcpy with
strscpy. The function strcpy() return value isn't used so there
shouldn't be an issue replacing with the safer alternative strscpy.

Signed-off-by: Daniel Yang <danielyangkang@gmail.com>
---
V1 -> V2: handle strscpy errors, changed prefix to f2fs

 fs/f2fs/super.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

 				break;
 			}
 
-			strcpy(ext[ext_cnt], name);
+			if (strscpy(ext[ext_cnt], name) == -E2BIG) {
+				kfree(name);
+				return -EINVAL;
+			}
 			F2FS_OPTION(sbi).compress_ext_cnt++;
 			kfree(name);
 			break;
@@ -1187,7 +1191,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 				break;
 			}
 
-			strcpy(noext[noext_cnt], name);
+			if (strscpy(noext[noext_cnt], name) == -E2BIG) {
+				kfree(name);
+				return -EINVAL;
+			}
 			F2FS_OPTION(sbi).nocompress_ext_cnt++;
 			kfree(name);
 			break;

Comments

Chao Yu Nov. 18, 2024, 1:23 a.m. UTC | #1
On 2024/11/9 9:38, Daniel Yang wrote:
> strcpy is deprecated. Kernel docs recommend replacing strcpy with
> strscpy. The function strcpy() return value isn't used so there
> shouldn't be an issue replacing with the safer alternative strscpy.
> 
> Signed-off-by: Daniel Yang <danielyangkang@gmail.com>
> ---
> V1 -> V2: handle strscpy errors, changed prefix to f2fs
> 
>   fs/f2fs/super.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 87ab5696b..4721a8a8f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -5,6 +5,7 @@
>    * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>    *             http://www.samsung.com/
>    */
>   #include <linux/module.h>
>   #include <linux/init.h>
>   #include <linux/fs.h>
> @@ -1158,7 +1159,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>   				break;
>   			}
>   
> -			strcpy(ext[ext_cnt], name);
> +			if (strscpy(ext[ext_cnt], name) == -E2BIG) {
> +				kfree(name);
> +				return -EINVAL;

How about?

ret = strscpy(ext[ext_cnt], name);
if (ret < 0) {
	kfree(name);
	return ret;
}

> +			}
>   			F2FS_OPTION(sbi).compress_ext_cnt++;
>   			kfree(name);
>   			break;
> @@ -1187,7 +1191,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>   				break;
>   			}
>   
> -			strcpy(noext[noext_cnt], name);
> +			if (strscpy(noext[noext_cnt], name) == -E2BIG) {
> +				kfree(name);
> +				return -EINVAL;
> +			}

Ditto

Thanks,

>   			F2FS_OPTION(sbi).nocompress_ext_cnt++;
>   			kfree(name);
>   			break;
Daniel Yang Nov. 18, 2024, 7:03 a.m. UTC | #2
On Sun, Nov 17, 2024 at 5:24 PM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/11/9 9:38, Daniel Yang wrote:
> > strcpy is deprecated. Kernel docs recommend replacing strcpy with
> > strscpy. The function strcpy() return value isn't used so there
> > shouldn't be an issue replacing with the safer alternative strscpy.
> >
> > Signed-off-by: Daniel Yang <danielyangkang@gmail.com>
> > ---
> > V1 -> V2: handle strscpy errors, changed prefix to f2fs
> >
> >   fs/f2fs/super.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 87ab5696b..4721a8a8f 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -5,6 +5,7 @@
> >    * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> >    *             http://www.samsung.com/
> >    */
> >   #include <linux/module.h>
> >   #include <linux/init.h>
> >   #include <linux/fs.h>
> > @@ -1158,7 +1159,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> >                               break;
> >                       }
> >
> > -                     strcpy(ext[ext_cnt], name);
> > +                     if (strscpy(ext[ext_cnt], name) == -E2BIG) {
> > +                             kfree(name);
> > +                             return -EINVAL;
>
> How about?
>
> ret = strscpy(ext[ext_cnt], name);
> if (ret < 0) {
>         kfree(name);
>         return ret;
> }
>
> > +                     }
> >                       F2FS_OPTION(sbi).compress_ext_cnt++;
> >                       kfree(name);
> >                       break;
> > @@ -1187,7 +1191,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> >                               break;
> >                       }
> >
> > -                     strcpy(noext[noext_cnt], name);
> > +                     if (strscpy(noext[noext_cnt], name) == -E2BIG) {
> > +                             kfree(name);
> > +                             return -EINVAL;
> > +                     }
>
> Ditto
>
> Thanks,
>
> >                       F2FS_OPTION(sbi).nocompress_ext_cnt++;
> >                       kfree(name);
> >                       break;
>

Thanks for the feedback. That does look much cleaner. I've applied the
changes and sent v3.

- Daniel
diff mbox series

Patch

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 87ab5696b..4721a8a8f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -5,6 +5,7 @@ 
  * Copyright (c) 2012 Samsung Electronics Co., Ltd.
  *             http://www.samsung.com/
  */
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/fs.h>
@@ -1158,7 +1159,10 @@  static int parse_options(struct super_block *sb, char *options, bool is_remount)