[LTP] [PATCH v1] libswap.c: Check free space with correct mnt path

[LTP] [PATCH v1] libswap.c: Check free space with correct mnt path

[Wei Gao via ltp]

The tst_fs_has_free should check fs size of mnt point.
But current code check ".", that means check /tmp/LTP_xxx
instead of /tmp/LTP_xxx/mntpoint.

Also tst_fs_has_free's "size" parameter's type is unsigned int,
it will overflow if encounter big filesystem block size(such as Btrfs
can use 64k).

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/tst_fs.h          | 6 +++---
 lib/tst_fs_has_free.c     | 2 +-
 libs/libltpswap/libswap.c | 4 +++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/tst_fs.h b/include/tst_fs.h
index 1dd7d32fc..99dbba4d1 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -60,7 +60,7 @@ enum {
  * @mult: mult should be TST_KB, TST_MB or TST_GB
  * the required free space is calculated by @size * @mult
  */
-int tst_fs_has_free_(void (*cleanup)(void), const char *path, unsigned int size,
+int tst_fs_has_free_(void (*cleanup)(void), const char *path, uint64_t size,
 		     unsigned int mult);
 
 /*
@@ -225,7 +225,7 @@ static inline long tst_fs_type(const char *path)
 	return tst_fs_type_(NULL, path);
 }
 
-static inline int tst_fs_has_free(const char *path, unsigned int size,
+static inline int tst_fs_has_free(const char *path, uint64_t size,
 				  unsigned int mult)
 {
 	return tst_fs_has_free_(NULL, path, size, mult);
@@ -252,7 +252,7 @@ static inline long tst_fs_type(void (*cleanup)(void), const char *path)
 }
 
 static inline int tst_fs_has_free(void (*cleanup)(void), const char *path,
-				  unsigned int size, unsigned int mult)
+				  uint64_t size, unsigned int mult)
 {
 	return tst_fs_has_free_(cleanup, path, size, mult);
 }
diff --git a/lib/tst_fs_has_free.c b/lib/tst_fs_has_free.c
index e82dfa837..080d693ab 100644
--- a/lib/tst_fs_has_free.c
+++ b/lib/tst_fs_has_free.c
@@ -27,7 +27,7 @@
 #include "tst_fs.h"
 
 int tst_fs_has_free_(void (*cleanup)(void), const char *path,
-		     unsigned int size, unsigned int mult)
+		     uint64_t size, unsigned int mult)
 {
 	struct statfs sf;
 
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index c8cbb8ea1..6c75d20fa 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -137,6 +137,7 @@ int make_swapfile(const char *swapfile, int blocks, int safe)
 	struct statvfs fs_info;
 	unsigned long blk_size, bs;
 	size_t pg_size = sysconf(_SC_PAGESIZE);
+	char mnt_path[100];
 
 	if (statvfs(".", &fs_info) == -1)
 		return -1;
@@ -149,7 +150,8 @@ int make_swapfile(const char *swapfile, int blocks, int safe)
 	else
 		bs = blk_size;
 
-	if (!tst_fs_has_free(".", bs * blocks, TST_BYTES))
+	sscanf(swapfile, "%[^/]", mnt_path);
+	if (!tst_fs_has_free(mnt_path, bs * blocks, TST_BYTES))
 		tst_brk(TCONF, "Insufficient disk space to create swap file");
 
 	/* create file */
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] libswap.c: Check free space with correct mnt path

[Li Wang]

Hi Wei,

Good catch!

Reviewed-by: Li Wang <liwang@redhat.com>


On Fri, Mar 1, 2024 at 9:51 AM Wei Gao via ltp <ltp@lists.linux.it> wrote:

> The tst_fs_has_free should check fs size of mnt point.
> But current code check ".", that means check /tmp/LTP_xxx
> instead of /tmp/LTP_xxx/mntpoint.
>
> Also tst_fs_has_free's "size" parameter's type is unsigned int,
> it will overflow if encounter big filesystem block size(such as Btrfs
> can use 64k).
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/tst_fs.h          | 6 +++---
>  lib/tst_fs_has_free.c     | 2 +-
>  libs/libltpswap/libswap.c | 4 +++-
>  3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/include/tst_fs.h b/include/tst_fs.h
> index 1dd7d32fc..99dbba4d1 100644
> --- a/include/tst_fs.h
> +++ b/include/tst_fs.h
> @@ -60,7 +60,7 @@ enum {
>   * @mult: mult should be TST_KB, TST_MB or TST_GB
>   * the required free space is calculated by @size * @mult
>   */
> -int tst_fs_has_free_(void (*cleanup)(void), const char *path, unsigned
> int size,
> +int tst_fs_has_free_(void (*cleanup)(void), const char *path, uint64_t
> size,
>                      unsigned int mult);
>
>  /*
> @@ -225,7 +225,7 @@ static inline long tst_fs_type(const char *path)
>         return tst_fs_type_(NULL, path);
>  }
>
> -static inline int tst_fs_has_free(const char *path, unsigned int size,
> +static inline int tst_fs_has_free(const char *path, uint64_t size,
>                                   unsigned int mult)
>  {
>         return tst_fs_has_free_(NULL, path, size, mult);
> @@ -252,7 +252,7 @@ static inline long tst_fs_type(void (*cleanup)(void),
> const char *path)
>  }
>
>  static inline int tst_fs_has_free(void (*cleanup)(void), const char *path,
> -                                 unsigned int size, unsigned int mult)
> +                                 uint64_t size, unsigned int mult)
>  {
>         return tst_fs_has_free_(cleanup, path, size, mult);
>  }
> diff --git a/lib/tst_fs_has_free.c b/lib/tst_fs_has_free.c
> index e82dfa837..080d693ab 100644
> --- a/lib/tst_fs_has_free.c
> +++ b/lib/tst_fs_has_free.c
> @@ -27,7 +27,7 @@
>  #include "tst_fs.h"
>
>  int tst_fs_has_free_(void (*cleanup)(void), const char *path,
> -                    unsigned int size, unsigned int mult)
> +                    uint64_t size, unsigned int mult)
>  {
>         struct statfs sf;
>
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index c8cbb8ea1..6c75d20fa 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -137,6 +137,7 @@ int make_swapfile(const char *swapfile, int blocks,
> int safe)
>         struct statvfs fs_info;
>         unsigned long blk_size, bs;
>         size_t pg_size = sysconf(_SC_PAGESIZE);
> +       char mnt_path[100];
>
>         if (statvfs(".", &fs_info) == -1)
>                 return -1;
> @@ -149,7 +150,8 @@ int make_swapfile(const char *swapfile, int blocks,
> int safe)
>         else
>                 bs = blk_size;
>
> -       if (!tst_fs_has_free(".", bs * blocks, TST_BYTES))
> +       sscanf(swapfile, "%[^/]", mnt_path);
> +       if (!tst_fs_has_free(mnt_path, bs * blocks, TST_BYTES))
>                 tst_brk(TCONF, "Insufficient disk space to create swap
> file");
>
>         /* create file */
> --
> 2.35.3
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] libswap.c: Check free space with correct mnt path

[Petr Vorel]

Hi Wei, Li,

> Hi Wei,

> Good catch!

Great, thanks, merged!
(Tested at least on intel.)

> Reviewed-by: Li Wang <liwang@redhat.com>


> On Fri, Mar 1, 2024 at 9:51 AM Wei Gao via ltp <ltp@lists.linux.it> wrote:

> > The tst_fs_has_free should check fs size of mnt point.
> > But current code check ".", that means check /tmp/LTP_xxx
> > instead of /tmp/LTP_xxx/mntpoint.

> > Also tst_fs_has_free's "size" parameter's type is unsigned int,
> > it will overflow if encounter big filesystem block size(such as Btrfs
> > can use 64k).

I would personally added this as separate previous patch, but kept as is.

...
> > +++ b/libs/libltpswap/libswap.c
> > @@ -137,6 +137,7 @@ int make_swapfile(const char *swapfile, int blocks,
> > int safe)
> >         struct statvfs fs_info;
> >         unsigned long blk_size, bs;
> >         size_t pg_size = sysconf(_SC_PAGESIZE);
> > +       char mnt_path[100];

> >         if (statvfs(".", &fs_info) == -1)
> >                 return -1;
> > @@ -149,7 +150,8 @@ int make_swapfile(const char *swapfile, int blocks,
> > int safe)
> >         else
> >                 bs = blk_size;

> > -       if (!tst_fs_has_free(".", bs * blocks, TST_BYTES))
> > +       sscanf(swapfile, "%[^/]", mnt_path);

Although this should never fail, I changed it to check return value:

-       if (!tst_fs_has_free(".", bs * blocks, TST_BYTES))
+       if (sscanf(swapfile, "%[^/]", mnt_path) != 1)
+               tst_brk(TBROK, "sscanf failed");

And I'll ask Andrea Manzini to add SAFE_SSCANF() (easy hack).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp