fs/ceph/export.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
strcpy() is generally considered unsafe and use of strscpy() is
recommended [1]
this fixes checkpatch warning:
WARNING: Prefer strscpy over strcpy
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
Signed-off-by: Abdul Rahim <abdul.rahim@myyahoo.com>
---
fs/ceph/export.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index 44451749c544..0e5b3c7b3756 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -452,7 +452,7 @@ static int __get_snap_name(struct dentry *parent, char *name,
goto out;
if (ceph_snap(inode) == CEPH_SNAPDIR) {
if (ceph_snap(dir) == CEPH_NOSNAP) {
- strcpy(name, fsc->mount_options->snapdir_name);
+ strscpy(name, fsc->mount_options->snapdir_name);
err = 0;
}
goto out;
--
2.43.0Hi Abdul,
kernel test robot noticed the following build errors:
[auto build test ERROR on ceph-client/testing]
[also build test ERROR on ceph-client/for-linus linus/master v6.12-rc7 next-20241112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Abdul-Rahim/Use-strscpy-instead-of-strcpy/20241112-064257
base: https://github.com/ceph/ceph-client.git testing
patch link: https://lore.kernel.org/r/20241111221037.92853-1-abdul.rahim%40myyahoo.com
patch subject: [PATCH] Use strscpy() instead of strcpy()
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241113/202411130853.c11sinW2-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241113/202411130853.c11sinW2-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411130853.c11sinW2-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/bitfield.h:10,
from include/linux/fortify-string.h:5,
from include/linux/string.h:390,
from include/linux/ceph/ceph_debug.h:7,
from fs/ceph/export.c:2:
fs/ceph/export.c: In function '__get_snap_name':
>> include/linux/build_bug.h:16:51: error: negative width in bit-field '<anonymous>'
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/compiler.h:243:33: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
243 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
| ^~~~~~~~~~~~~~~~~
include/linux/string.h:79:47: note: in expansion of macro '__must_be_array'
79 | sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst) + \
| ^~~~~~~~~~~~~~~
include/linux/args.h:25:24: note: in expansion of macro '__strscpy0'
25 | #define __CONCAT(a, b) a ## b
| ^
include/linux/args.h:26:27: note: in expansion of macro '__CONCAT'
26 | #define CONCATENATE(a, b) __CONCAT(a, b)
| ^~~~~~~~
include/linux/string.h:113:9: note: in expansion of macro 'CONCATENATE'
113 | CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__)
| ^~~~~~~~~~~
fs/ceph/export.c:455:25: note: in expansion of macro 'strscpy'
455 | strscpy(name, fsc->mount_options->snapdir_name);
| ^~~~~~~
vim +16 include/linux/build_bug.h
bc6245e5efd70c Ian Abbott 2017-07-10 6
bc6245e5efd70c Ian Abbott 2017-07-10 7 #ifdef __CHECKER__
bc6245e5efd70c Ian Abbott 2017-07-10 8 #define BUILD_BUG_ON_ZERO(e) (0)
bc6245e5efd70c Ian Abbott 2017-07-10 9 #else /* __CHECKER__ */
bc6245e5efd70c Ian Abbott 2017-07-10 10 /*
bc6245e5efd70c Ian Abbott 2017-07-10 11 * Force a compilation error if condition is true, but also produce a
8788994376d84d Rikard Falkeborn 2019-12-04 12 * result (of value 0 and type int), so the expression can be used
bc6245e5efd70c Ian Abbott 2017-07-10 13 * e.g. in a structure initializer (or where-ever else comma expressions
bc6245e5efd70c Ian Abbott 2017-07-10 14 * aren't permitted).
bc6245e5efd70c Ian Abbott 2017-07-10 15 */
8788994376d84d Rikard Falkeborn 2019-12-04 @16 #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
527edbc18a70e7 Masahiro Yamada 2019-01-03 17 #endif /* __CHECKER__ */
527edbc18a70e7 Masahiro Yamada 2019-01-03 18
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Le 11/11/2024 à 23:10, Abdul Rahim a écrit :
> strcpy() is generally considered unsafe and use of strscpy() is
> recommended [1]
>
> this fixes checkpatch warning:
> WARNING: Prefer strscpy over strcpy
>
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
> Signed-off-by: Abdul Rahim <abdul.rahim@myyahoo.com>
> ---
> fs/ceph/export.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index 44451749c544..0e5b3c7b3756 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -452,7 +452,7 @@ static int __get_snap_name(struct dentry *parent, char *name,
> goto out;
> if (ceph_snap(inode) == CEPH_SNAPDIR) {
> if (ceph_snap(dir) == CEPH_NOSNAP) {
> - strcpy(name, fsc->mount_options->snapdir_name);
> + strscpy(name, fsc->mount_options->snapdir_name);
This does not compile because when the size of 'name' is not known at
compilation time, you need to use the 3-argument version of strscpy().
Please always compile test your patches before sending them. Even, when
the change looks trivial.
CJ
> err = 0;
> }
> goto out;
On Tue, Nov 12, 2024 at 07:21:54AM +0100, Christophe JAILLET wrote:
> Le 11/11/2024 � 23:10, Abdul Rahim a �crit�:
> > strcpy() is generally considered unsafe and use of strscpy() is
> > recommended [1]
> >
> > this fixes checkpatch warning:
> > WARNING: Prefer strscpy over strcpy
> >
> > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
> > Signed-off-by: Abdul Rahim <abdul.rahim@myyahoo.com>
> > ---
> > fs/ceph/export.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> > index 44451749c544..0e5b3c7b3756 100644
> > --- a/fs/ceph/export.c
> > +++ b/fs/ceph/export.c
> > @@ -452,7 +452,7 @@ static int __get_snap_name(struct dentry *parent, char *name,
> > goto out;
> > if (ceph_snap(inode) == CEPH_SNAPDIR) {
> > if (ceph_snap(dir) == CEPH_NOSNAP) {
> > - strcpy(name, fsc->mount_options->snapdir_name);
> > + strscpy(name, fsc->mount_options->snapdir_name);
>
> This does not compile because when the size of 'name' is not known at
> compilation time, you need to use the 3-argument version of strscpy().
>
> Please always compile test your patches before sending them. Even, when the
> change looks trivial.
>
Sure.
> CJ
>
> > err = 0;
> > }
> > goto out;
>
>
Should it be: NAME_MAX+1 ?
See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/exportfs.h?h=v6.12-rc7#n203
Le 13/11/2024 à 21:15, Abdul Rahim a écrit :
> On Tue, Nov 12, 2024 at 07:21:54AM +0100, Christophe JAILLET wrote:
>> Le 11/11/2024 � 23:10, Abdul Rahim a �crit�:
>>> strcpy() is generally considered unsafe and use of strscpy() is
>>> recommended [1]
>>>
>>> this fixes checkpatch warning:
>>> WARNING: Prefer strscpy over strcpy
>>>
>>> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
>>> Signed-off-by: Abdul Rahim <abdul.rahim@myyahoo.com>
>>> ---
>>> fs/ceph/export.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
>>> index 44451749c544..0e5b3c7b3756 100644
>>> --- a/fs/ceph/export.c
>>> +++ b/fs/ceph/export.c
>>> @@ -452,7 +452,7 @@ static int __get_snap_name(struct dentry *parent, char *name,
>>> goto out;
>>> if (ceph_snap(inode) == CEPH_SNAPDIR) {
>>> if (ceph_snap(dir) == CEPH_NOSNAP) {
>>> - strcpy(name, fsc->mount_options->snapdir_name);
>>> + strscpy(name, fsc->mount_options->snapdir_name);
>>
>> This does not compile because when the size of 'name' is not known at
>> compilation time, you need to use the 3-argument version of strscpy().
>>
>> Please always compile test your patches before sending them. Even, when the
>> change looks trivial.
>>
>
> Sure.
>
>> CJ
>>
>>> err = 0;
>>> }
>>> goto out;
>>
>>
>
> Should it be: NAME_MAX+1 ?
>
> See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/exportfs.h?h=v6.12-rc7#n203
>
>
Looks like a good idea, maybe with a comment related to get_name() in
the export_operations structure, so that we remind where this limit
comes from?
CJ
On Wed, Nov 13, 2024 at 10:28:36PM +0100, Christophe JAILLET wrote:
> Le 13/11/2024 � 21:15, Abdul Rahim a �crit�:
> > On Tue, Nov 12, 2024 at 07:21:54AM +0100, Christophe JAILLET wrote:
> > > Le 11/11/2024 � 23:10, Abdul Rahim a �crit�:
> > > > strcpy() is generally considered unsafe and use of strscpy() is
> > > > recommended [1]
> > > >
> > > > this fixes checkpatch warning:
> > > > WARNING: Prefer strscpy over strcpy
> > > >
> > > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
> > > > Signed-off-by: Abdul Rahim <abdul.rahim@myyahoo.com>
> > > > ---
> > > > fs/ceph/export.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> > > > index 44451749c544..0e5b3c7b3756 100644
> > > > --- a/fs/ceph/export.c
> > > > +++ b/fs/ceph/export.c
> > > > @@ -452,7 +452,7 @@ static int __get_snap_name(struct dentry *parent, char *name,
> > > > goto out;
> > > > if (ceph_snap(inode) == CEPH_SNAPDIR) {
> > > > if (ceph_snap(dir) == CEPH_NOSNAP) {
> > > > - strcpy(name, fsc->mount_options->snapdir_name);
> > > > + strscpy(name, fsc->mount_options->snapdir_name);
> > >
> > > This does not compile because when the size of 'name' is not known at
> > > compilation time, you need to use the 3-argument version of strscpy().
> > >
> > > Please always compile test your patches before sending them. Even, when the
> > > change looks trivial.
> > >
> >
> > Sure.
> >
> > > CJ
> > >
> > > > err = 0;
> > > > }
> > > > goto out;
> > >
> >
> > Should it be: NAME_MAX+1 ?
> >
> > See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/exportfs.h?h=v6.12-rc7#n203
> >
> >
>
> Looks like a good idea, maybe with a comment related to get_name() in the
> export_operations structure, so that we remind where this limit comes from?
>
> CJ
>
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index 0e5b3c7b3756..48265c879fcf 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -452,7 +452,12 @@ static int __get_snap_name(struct dentry *parent, char *name,
goto out;
if (ceph_snap(inode) == CEPH_SNAPDIR) {
if (ceph_snap(dir) == CEPH_NOSNAP) {
- strcpy(name, fsc->mount_options->snapdir_name);
+ /*
+ * get_name assumes that name is pointing to a
+ * NAME_MAX+1 sized buffer
+ */
+ strscpy(name, fsc->mount_options->snapdir_name,
+ NAME_MAX+1);
err = 0;
}
goto out;
Looks good?
Le 14/11/2024 à 10:14, Abdul Rahim a écrit :
> On Wed, Nov 13, 2024 at 10:28:36PM +0100, Christophe JAILLET wrote:
...
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index 0e5b3c7b3756..48265c879fcf 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -452,7 +452,12 @@ static int __get_snap_name(struct dentry *parent, char *name,
> goto out;
> if (ceph_snap(inode) == CEPH_SNAPDIR) {
> if (ceph_snap(dir) == CEPH_NOSNAP) {
> - strcpy(name, fsc->mount_options->snapdir_name);
> + /*
> + * get_name assumes that name is pointing to a
> + * NAME_MAX+1 sized buffer
> + */
It is a matter of taste, and I'm not the maintainer, but my personal
feeling would go for something like:
/* .get_name() from struct export_operations assumes that its 'name'
parameter is pointing to a NAME_MAX+1 sized buffer */
CJ
> + strscpy(name, fsc->mount_options->snapdir_name,
> + NAME_MAX+1);
> err = 0;
> }
> goto out;
>
>
> Looks good?
>
>
On Thu, Nov 14, 2024 at 09:53:46PM +0100, Christophe JAILLET wrote:
> Le 14/11/2024 à 10:14, Abdul Rahim a écrit :
> > On Wed, Nov 13, 2024 at 10:28:36PM +0100, Christophe JAILLET wrote:
>
> ...
>
> > diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> > index 0e5b3c7b3756..48265c879fcf 100644
> > --- a/fs/ceph/export.c
> > +++ b/fs/ceph/export.c
> > @@ -452,7 +452,12 @@ static int __get_snap_name(struct dentry *parent, char *name,
> > goto out;
> > if (ceph_snap(inode) == CEPH_SNAPDIR) {
> > if (ceph_snap(dir) == CEPH_NOSNAP) {
> > - strcpy(name, fsc->mount_options->snapdir_name);
> > + /*
> > + * get_name assumes that name is pointing to a
> > + * NAME_MAX+1 sized buffer
> > + */
>
> It is a matter of taste, and I'm not the maintainer, but my personal feeling
> would go for something like:
>
> /* .get_name() from struct export_operations assumes that its 'name'
> parameter is pointing to a NAME_MAX+1 sized buffer */
>
> CJ
https://lore.kernel.org/lkml/20241115112419.11137-1-abdul.rahim@myyahoo.com/
© 2016 - 2026 Red Hat, Inc.