| Message ID | 20221124114204.990-1-akumar@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | statvfs01: Convert to new LTP API | expand |
On Thu, Nov 24, 2022 at 7:42 PM Avinesh Kumar <akumar@suse.de> wrote: > Also I've removed the TINFO statements, I'm not sure if only > printing the data in logs is helpful in anyway. > Removing the printing is OK, but I am just wondering that can we find a way to check if the returned value in 'buf' is indeed correct? As you can see the ‘struct statvfs‘ includes many fs key values that we need to trust when using them. struct statvfs { unsigned long f_bsize; /* Filesystem block size */ unsigned long f_frsize; /* Fragment size */ fsblkcnt_t f_blocks; /* Size of fs in f_frsize units */ fsblkcnt_t f_bfree; /* Number of free blocks */ fsblkcnt_t f_bavail; /* Number of free blocks for unprivileged users */ fsfilcnt_t f_files; /* Number of inodes */ fsfilcnt_t f_ffree; /* Number of free inodes */ fsfilcnt_t f_favail; /* Number of free inodes for unprivileged users */ unsigned long f_fsid; /* Filesystem ID */ unsigned long f_flag; /* Mount flags */ unsigned long f_namemax; /* Maximum filename length */ }; > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > --- > testcases/kernel/syscalls/statvfs/statvfs01.c | 96 +++++-------------- > 1 file changed, 22 insertions(+), 74 deletions(-) > > diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c > b/testcases/kernel/syscalls/statvfs/statvfs01.c > index e3b356c93..89ca4e960 100644 > --- a/testcases/kernel/syscalls/statvfs/statvfs01.c > +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c > @@ -1,92 +1,40 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. > * AUTHOR: Prashant P Yendigeri <prashant.yendigeri@wipro.com> > - * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms of version 2 of the GNU General Public License as > - * published by the Free Software Foundation. > - * > - * This program is distributed in the hope that it would be useful, but > - * WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > - * > - * You should have received a copy of the GNU General Public License along > - * with this program; if not, write the Free Software Foundation, Inc., > - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > - * > + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com> > */ > -/* > - * DESCRIPTION > - * This is a Phase I test for the statvfs(2) system call. > - * It is intended to provide a limited exposure of the system call. > - * This call behaves similar to statfs. > + > +/*\ > + * [Description] > + * > + * Verify that statvfs() executes successfully for all > + * available filesystems. > */ > > -#include <stdio.h> > -#include <unistd.h> > -#include <errno.h> > #include <sys/statvfs.h> > -#include <stdint.h> > - > -#include "test.h" > - > -#define TEST_PATH "/" > +#include "tst_test.h" > > -static void setup(void); > -static void cleanup(void); > +#define MNT_POINT "mntpoint" > +#define TEST_PATH MNT_POINT"/testfile" > > -char *TCID = "statvfs01"; > -int TST_TOTAL = 1; > - > -int main(int ac, char **av) > +static void run(void) > { > struct statvfs buf; > - int lc; > - > - tst_parse_opts(ac, av, NULL, NULL); > > - setup(); > - > - for (lc = 0; TEST_LOOPING(lc); lc++) { > - > - tst_count = 0; > - > - TEST(statvfs(TEST_PATH, &buf)); > - > - if (TEST_RETURN == -1) { > - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) > failed", > - TEST_PATH); > - } else { > - tst_resm(TPASS, "statvfs(%s, ...) passed", > TEST_PATH); > - } > - > - } > - > - tst_resm(TINFO, "This call is similar to statfs"); > - tst_resm(TINFO, "Extracting info about the '%s' file system", > - TEST_PATH); > - tst_resm(TINFO, "file system block size = %lu bytes", buf.f_bsize); > - tst_resm(TINFO, "file system fragment size = %lu bytes", > buf.f_frsize); > - tst_resm(TINFO, "file system free blocks = %ju", > - (uintmax_t) buf.f_bfree); > - tst_resm(TINFO, "file system total inodes = %ju", > - (uintmax_t) buf.f_files); > - tst_resm(TINFO, "file system free inodes = %ju", > - (uintmax_t) buf.f_ffree); > - tst_resm(TINFO, "file system id = %lu", buf.f_fsid); > - tst_resm(TINFO, "file system max filename length = %lu", > buf.f_namemax); > - > - cleanup(); > - tst_exit(); > + TST_EXP_PASS(statvfs(TEST_PATH, &buf)); > } > > static void setup(void) > { > - tst_sig(NOFORK, DEF_HANDLER, cleanup); > - > - TEST_PAUSE; > + SAFE_TOUCH(TEST_PATH, 0666, NULL); > } > > -static void cleanup(void) > -{ > -} > +static struct tst_test test = { > + .test_all = run, > + .setup = setup, > + .needs_root = 1, > + .mount_device = 1, > + .mntpoint = MNT_POINT, > + .all_filesystems = 1 > +}; > -- > 2.38.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > >
Hello, Li Wang <liwang@redhat.com> writes: > On Thu, Nov 24, 2022 at 7:42 PM Avinesh Kumar <akumar@suse.de> wrote: > > Also I've removed the TINFO statements, I'm not sure if only > printing the data in logs is helpful in anyway. > > Removing the printing is OK, but I am just wondering that > can we find a way to check if the returned value in 'buf' is > indeed correct? > > As you can see the ‘struct statvfs‘ includes many fs key > values that we need to trust when using them. > > struct statvfs { > unsigned long f_bsize; /* Filesystem block size */ > unsigned long f_frsize; /* Fragment size */ > fsblkcnt_t f_blocks; /* Size of fs in f_frsize units */ > fsblkcnt_t f_bfree; /* Number of free blocks */ > fsblkcnt_t f_bavail; /* Number of free blocks for > unprivileged users */ > fsfilcnt_t f_files; /* Number of inodes */ > fsfilcnt_t f_ffree; /* Number of free inodes */ > fsfilcnt_t f_favail; /* Number of free inodes for > unprivileged users */ > unsigned long f_fsid; /* Filesystem ID */ > unsigned long f_flag; /* Mount flags */ > unsigned long f_namemax; /* Maximum filename length */ > }; I suppose previously printing the values at least accessed the memory. Actually validating the values could be a separate patch though. We (probably) know that maximum filename should be less than 255 chars (for e.g.), but I think there is a good chance that trying to validate this will result in false positives and stuff we might want to revert.
On Tue, Nov 29, 2022 at 7:09 PM Richard Palethorpe <rpalethorpe@suse.de> wrote: > Hello, > > Li Wang <liwang@redhat.com> writes: > > > On Thu, Nov 24, 2022 at 7:42 PM Avinesh Kumar <akumar@suse.de> wrote: > > > > Also I've removed the TINFO statements, I'm not sure if only > > printing the data in logs is helpful in anyway. > > > > Removing the printing is OK, but I am just wondering that > > can we find a way to check if the returned value in 'buf' is > > indeed correct? > > > > As you can see the ‘struct statvfs‘ includes many fs key > > values that we need to trust when using them. > > > > struct statvfs { > > unsigned long f_bsize; /* Filesystem block size */ > > unsigned long f_frsize; /* Fragment size */ > > fsblkcnt_t f_blocks; /* Size of fs in f_frsize units */ > > fsblkcnt_t f_bfree; /* Number of free blocks */ > > fsblkcnt_t f_bavail; /* Number of free blocks for > > unprivileged users */ > > fsfilcnt_t f_files; /* Number of inodes */ > > fsfilcnt_t f_ffree; /* Number of free inodes */ > > fsfilcnt_t f_favail; /* Number of free inodes for > > unprivileged users */ > > unsigned long f_fsid; /* Filesystem ID */ > > unsigned long f_flag; /* Mount flags */ > > unsigned long f_namemax; /* Maximum filename length */ > > }; > > I suppose previously printing the values at least accessed the memory. > Sounds reasonable. > Actually validating the values could be a separate patch though. > +1 Absolutely. > > We (probably) know that maximum filename should be less than 255 chars > (for e.g.), but I think there is a good chance that trying to validate > this will result in false positives and stuff we might want to revert. > Maybe we can create a concrete size of the device and mount it with a designated FS (e.g. ext4), then extracting the known FS data into `struct statvfs` and validating them. Does this make sense?
Li Wang <liwang@redhat.com> wrote: > We (probably) know that maximum filename should be less than 255 chars >> (for e.g.), but I think there is a good chance that trying to validate >> this will result in false positives and stuff we might want to revert. >> > > Maybe we can create a concrete size of the device and mount > it with a designated FS (e.g. ext4), then extracting the known FS > data into `struct statvfs` and validating them. > As some of the data are determined by the FS attribute and device size. (e.g. block size, total block, fragment size, and total inodes). We can check them directly by known values. For those easy change data, e.g. FS free blocks, which determined by the real system status, maybe just check that is no large than the total block number. Based on that I draft something below to validate the fields. what do you think? #include <sys/statvfs.h> #include "tst_test.h" #define MNT_POINT "mntpoint" #define TEST_PATH MNT_POINT"/testfile" static void run(void) { int type; struct statfs buf; struct statvfs vbuf; TST_EXP_PASS_SILENT(statfs(TEST_PATH, &buf)); TST_EXP_PASS(statvfs(TEST_PATH, &vbuf)); tst_res(TINFO, "Extracting FS info from the '%s' file", MNT_POINT); tst_res(TINFO, "vbuf.f_fsid == %lu", vbuf.f_fsid); long fs_type = tst_fs_type(TEST_PATH); switch (fs_type) { case TST_EXT2_OLD_MAGIC: case TST_EXT234_MAGIC: TST_EXP_EQ_LI(vbuf.f_bsize, 1024); TST_EXP_EQ_LI(vbuf.f_frsize, 1024); ttype = (vbuf.f_bfree <= buf.f_blocks) ? TPASS : TFAIL; tst_res(ttype, "vbuf.f_bfree == %ju", (uintmax_t) vbuf.f_bfree); TST_EXP_EQ_LI(vbuf.f_files, 76912); ttype = (vbuf.f_ffree <= vbuf.f_files) ? TPASS : TFAIL; tst_res(ttype, "vbuf.f_ffree == %ju", (uintmax_t) vbuf.f_ffree); TST_EXP_EQ_LI(vbuf.f_flag, 4096); TST_EXP_EQ_LI(vbuf.f_namemax, 255); break; case TST_XFS_MAGIC: TST_EXP_EQ_LI(vbuf.f_bsize, 4096); TST_EXP_EQ_LI(vbuf.f_frsize, 4096); ttype = (vbuf.f_bfree <= buf.f_blocks) ? TPASS : TFAIL; tst_res(ttype, "vbuf.f_bfree == %ju", (uintmax_t) vbuf.f_bfree); TST_EXP_EQ_LI(vbuf.f_files, 153600); ttype = (vbuf.f_ffree <= vbuf.f_files) ? TPASS : TFAIL; tst_res(ttype, "vbuf.f_ffree == %ju", (uintmax_t) vbuf.f_ffree); TST_EXP_EQ_LI(vbuf.f_flag, 4096); TST_EXP_EQ_LI(vbuf.f_namemax, 255); break; case TST_BTRFS_MAGIC: TST_EXP_EQ_LI(vbuf.f_bsize, 4096); TST_EXP_EQ_LI(vbuf.f_frsize, 4096); ttype = (vbuf.f_bfree <= buf.f_blocks) ? TPASS : TFAIL; tst_res(ttype, "vbuf.f_bfree == %ju", (uintmax_t) vbuf.f_bfree); TST_EXP_EQ_LI(vbuf.f_files, 0); ttype = (vbuf.f_ffree <= vbuf.f_files) ? TPASS : TFAIL; tst_res(ttype, "vbuf.f_ffree == %ju", (uintmax_t) vbuf.f_ffree); TST_EXP_EQ_LI(vbuf.f_flag, 4096); TST_EXP_EQ_LI(vbuf.f_namemax, 255); break; default: tst_res(TINFO, "vbuf.f_bsize == %lu bytes", vbuf.f_bsize); tst_res(TINFO, "vbuf.f_frsize == %lu bytes", vbuf.f_frsize); tst_res(TINFO, "vbuf.f_bfree == %ju", (uintmax_t) vbuf.f_bfree); tst_res(TINFO, "vbuf.f_files == %ju", (uintmax_t) vbuf.f_files); tst_res(TINFO, "vbuf.f_ffree == %ju", (uintmax_t) vbuf.f_ffree); tst_res(TINFO, "vbuf.f_namemax == %lu", vbuf.f_namemax); break; } } static void setup(void) { SAFE_TOUCH(TEST_PATH, 0666, NULL); } static struct tst_test test = { .test_all = run, .setup = setup, .needs_root = 1, .mount_device = 1, .mntpoint = MNT_POINT, .dev_min_size = 300, .all_filesystems = 1 };
diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c index e3b356c93..89ca4e960 100644 --- a/testcases/kernel/syscalls/statvfs/statvfs01.c +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c @@ -1,92 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. * AUTHOR: Prashant P Yendigeri <prashant.yendigeri@wipro.com> - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it would be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com> */ -/* - * DESCRIPTION - * This is a Phase I test for the statvfs(2) system call. - * It is intended to provide a limited exposure of the system call. - * This call behaves similar to statfs. + +/*\ + * [Description] + * + * Verify that statvfs() executes successfully for all + * available filesystems. */ -#include <stdio.h> -#include <unistd.h> -#include <errno.h> #include <sys/statvfs.h> -#include <stdint.h> - -#include "test.h" - -#define TEST_PATH "/" +#include "tst_test.h" -static void setup(void); -static void cleanup(void); +#define MNT_POINT "mntpoint" +#define TEST_PATH MNT_POINT"/testfile" -char *TCID = "statvfs01"; -int TST_TOTAL = 1; - -int main(int ac, char **av) +static void run(void) { struct statvfs buf; - int lc; - - tst_parse_opts(ac, av, NULL, NULL); - setup(); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - - tst_count = 0; - - TEST(statvfs(TEST_PATH, &buf)); - - if (TEST_RETURN == -1) { - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed", - TEST_PATH); - } else { - tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH); - } - - } - - tst_resm(TINFO, "This call is similar to statfs"); - tst_resm(TINFO, "Extracting info about the '%s' file system", - TEST_PATH); - tst_resm(TINFO, "file system block size = %lu bytes", buf.f_bsize); - tst_resm(TINFO, "file system fragment size = %lu bytes", buf.f_frsize); - tst_resm(TINFO, "file system free blocks = %ju", - (uintmax_t) buf.f_bfree); - tst_resm(TINFO, "file system total inodes = %ju", - (uintmax_t) buf.f_files); - tst_resm(TINFO, "file system free inodes = %ju", - (uintmax_t) buf.f_ffree); - tst_resm(TINFO, "file system id = %lu", buf.f_fsid); - tst_resm(TINFO, "file system max filename length = %lu", buf.f_namemax); - - cleanup(); - tst_exit(); + TST_EXP_PASS(statvfs(TEST_PATH, &buf)); } static void setup(void) { - tst_sig(NOFORK, DEF_HANDLER, cleanup); - - TEST_PAUSE; + SAFE_TOUCH(TEST_PATH, 0666, NULL); } -static void cleanup(void) -{ -} +static struct tst_test test = { + .test_all = run, + .setup = setup, + .needs_root = 1, + .mount_device = 1, + .mntpoint = MNT_POINT, + .all_filesystems = 1 +};
Also I've removed the TINFO statements, I'm not sure if only printing the data in logs is helpful in anyway. Signed-off-by: Avinesh Kumar <akumar@suse.de> --- testcases/kernel/syscalls/statvfs/statvfs01.c | 96 +++++-------------- 1 file changed, 22 insertions(+), 74 deletions(-)