diff mbox series

syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size

Message ID 1620372638-25690-1-git-send-email-xuyang2018.jy@fujitsu.com
State Changes Requested
Headers show
Series syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size | expand

Commit Message

xuyang2018.jy@fujitsu.com May 7, 2021, 7:30 a.m. UTC
Since these members of mallinfo struct use int data type, it will overflow
when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
in the future. So we test mallinfo2 whether works well.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                                  |  1 +
 runtest/syscalls                              |  2 +
 .../syscalls/mallinfo/mallinfo_common.h       | 18 +++++++
 .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
 testcases/kernel/syscalls/mallinfo2/Makefile  |  8 ++++
 .../kernel/syscalls/mallinfo2/mallinfo201.c   | 47 +++++++++++++++++++
 6 files changed, 77 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
 create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
 create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo201.c

Comments

Petr Vorel May 25, 2021, 10:56 a.m. UTC | #1
Hi Xu,

> Since these members of mallinfo struct use int data type, it will overflow
> when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
> in the future. So we test mallinfo2 whether works well.

> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  configure.ac                                  |  1 +
>  runtest/syscalls                              |  2 +
>  .../syscalls/mallinfo/mallinfo_common.h       | 18 +++++++
>  .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
>  testcases/kernel/syscalls/mallinfo2/Makefile  |  8 ++++
>  .../kernel/syscalls/mallinfo2/mallinfo201.c   | 47 +++++++++++++++++++
nit: Maybe rename test: mallinfo201.c => mallinfo2_01.c ?
Now it looks like mallinfo() 201st test.

>  6 files changed, 77 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
>  create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
>  create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo201.c

...
> diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> index a00cc7a64..e7737b270 100644
> --- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> @@ -28,4 +28,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
>  }
>  #endif

> +#ifdef HAVE_MALLINFO2
> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
> +{
> +	tst_res(TINFO, "%s...", msg);
> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
> +	P2(arena);
> +	P2(ordblks);
> +	P2(smblks);
> +	P2(hblks);
> +	P2(hblkhd);
> +	P2(usmblks);
> +	P2(fsmblks);
> +	P2(uordblks);
> +	P2(fordblks);
> +	P2(keepcost);
> +}
> +#endif

BTW did you copy this from glibc? (and previously in 7eb7a97a1)?
Code is similar as that one in malloc/tst-mallinfo2.c in glibc,
which you modified.

I guess we should also add glibc copyright to mallinfo_common.h
Copyright (C) 2020 Free Software Foundation, Inc.
(+ probably keep our)

...
> diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
> new file mode 100644
> index 000000000..044619fb8
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) International Business Machines  Corp., 2001
Here should be your or LTP copyright with 2021.

> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
CFLAGS += -I../mallinfo
(see description below)

> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
...
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [DESCRIPTION]
> + *
> + * Basic mallinfo2() test. Test the member of struct mallinfo2
> + * whether overflow when setting 2G size. mallinfo() is deprecated
> + * since the type used for the fields is too small. We should use
> + * mallinfo2() and it was added since glibc 2.33.
> + */

I'd format it a bit and remove info which is not related to the test and people
can find in man page.

/*\
 * [DESCRIPTION]
 *
 * Basic mallinfo2() test.
 *
 * Test members of struct mallinfo2() whether overflow when setting 2G size.
 */

> +#include "../mallinfo/mallinfo_common.h"
Please use
#include "mallinfo_common.h"

and add include path to to Makefile
CFLAGS += -I../mallinfo

> +#include "tst_safe_macros.h"
> +
> +#ifdef HAVE_MALLINFO2
> +
> +void test_mallinfo2(void)
> +{
> +	struct mallinfo2 info;
> +	char *buf;
> +	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
> +
> +	buf = malloc(size);
> +	if (!buf) {
> +		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
> +		return;
> +	}
BTW sometimes I wonder if we should have something like SAFE_*() functions which
would result with TCONF, not with TBROK.

> +	info = mallinfo2();
> +	if (info.hblkhd < size) {
> +		print_mallinfo2("Test malloc 2G", &info);
> +		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
> +	}
> +	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
> +	free(buf);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = test_mallinfo2,
> +};
> +
> +#else
> +TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
> +#endif

The rest LGTM.

Kind regards,
Petr
xuyang2018.jy@fujitsu.com June 3, 2021, 8:49 a.m. UTC | #2
Hi Petr
Sorry for late reply. I am busy recently.
> Hi Xu,
>
>> Since these members of mallinfo struct use int data type, it will overflow
>> when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
>> in the future. So we test mallinfo2 whether works well.
>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>>   configure.ac                                  |  1 +
>>   runtest/syscalls                              |  2 +
>>   .../syscalls/mallinfo/mallinfo_common.h       | 18 +++++++
>>   .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
>>   testcases/kernel/syscalls/mallinfo2/Makefile  |  8 ++++
>>   .../kernel/syscalls/mallinfo2/mallinfo201.c   | 47 +++++++++++++++++++
> nit: Maybe rename test: mallinfo201.c =>  mallinfo2_01.c ?
> Now it looks like mallinfo() 201st test.
Agree.

>>   6 files changed, 77 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
>>   create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
>>   create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo201.c
>
> ...
>> diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> index a00cc7a64..e7737b270 100644
>> --- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> @@ -28,4 +28,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
>>   }
>>   #endif
>
>> +#ifdef HAVE_MALLINFO2
>> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
>> +{
>> +	tst_res(TINFO, "%s...", msg);
>> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
>> +	P2(arena);
>> +	P2(ordblks);
>> +	P2(smblks);
>> +	P2(hblks);
>> +	P2(hblkhd);
>> +	P2(usmblks);
>> +	P2(fsmblks);
>> +	P2(uordblks);
>> +	P2(fordblks);
>> +	P2(keepcost);
>> +}
>> +#endif
>
> BTW did you copy this from glibc? (and previously in 7eb7a97a1)?
Yes.

> Code is similar as that one in malloc/tst-mallinfo2.c in glibc,
> which you modified.
>
> I guess we should also add glibc copyright to mallinfo_common.h
> Copyright (C) 2020 Free Software Foundation, Inc.
> (+ probably keep our)
I will add glibc copyright.
>
> ...
>> diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
>> new file mode 100644
>> index 000000000..044619fb8
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile
>> @@ -0,0 +1,8 @@
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# Copyright (c) International Business Machines  Corp., 2001
> Here should be your or LTP copyright with 2021.
>
OK.
>> +
>> +top_srcdir		?= ../../../..
>> +
>> +include $(top_srcdir)/include/mk/testcases.mk
> CFLAGS += -I../mallinfo
> (see description below)
>
>> +
>> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> ...
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
>> @@ -0,0 +1,47 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu<xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [DESCRIPTION]
>> + *
>> + * Basic mallinfo2() test. Test the member of struct mallinfo2
>> + * whether overflow when setting 2G size. mallinfo() is deprecated
>> + * since the type used for the fields is too small. We should use
>> + * mallinfo2() and it was added since glibc 2.33.
>> + */
>
> I'd format it a bit and remove info which is not related to the test and people
> can find in man page.
>
> /*\
>   * [DESCRIPTION]
>   *
>   * Basic mallinfo2() test.
>   *
>   * Test members of struct mallinfo2() whether overflow when setting 2G size.
>   */
>
Looks well.
>> +#include "../mallinfo/mallinfo_common.h"
> Please use
> #include "mallinfo_common.h"
>
> and add include path to to Makefile
> CFLAGS += -I../mallinfo
>
OK.
>> +#include "tst_safe_macros.h"
>> +
>> +#ifdef HAVE_MALLINFO2
>> +
>> +void test_mallinfo2(void)
>> +{
>> +	struct mallinfo2 info;
>> +	char *buf;
>> +	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
>> +
>> +	buf = malloc(size);
>> +	if (!buf) {
>> +		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
>> +		return;
>> +	}
> BTW sometimes I wonder if we should have something like SAFE_*() functions which
> would result with TCONF, not with TBROK.
IMO, I don't want to add a flag in SAFE_* functions to control return 
TCONF or TBORK because it is clear enough now. Other maintainers may 
have different option. But Now, I want to keep it.
>
>> +	info = mallinfo2();
>> +	if (info.hblkhd<  size) {
>> +		print_mallinfo2("Test malloc 2G",&info);
>> +		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
>> +	}
>> +	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
>> +	free(buf);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test_all = test_mallinfo2,
>> +};
>> +
>> +#else
>> +TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
>> +#endif
>
> The rest LGTM.
Thanks for your review. I will send a v2 patch soon.
>
> Kind regards,
> Petr
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 136d82d09..eb675b367 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,6 +102,7 @@  AC_CHECK_FUNCS_ONCE([ \
     io_uring_enter \
     kcmp \
     mallinfo \
+    mallinfo2 \
     mallopt \
     mkdirat \
     mknodat \
diff --git a/runtest/syscalls b/runtest/syscalls
index aa7fa24dd..ea48efd26 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -693,6 +693,8 @@  lstat02_64 lstat02_64
 mallinfo01 mallinfo01
 mallinfo02 mallinfo02
 
+mallinfo201 mallinfo201
+
 mallopt01 mallopt01
 
 mbind01 mbind01
diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
index a00cc7a64..e7737b270 100644
--- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
+++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
@@ -28,4 +28,22 @@  static inline void print_mallinfo(const char *msg, struct mallinfo *m)
 }
 #endif
 
+#ifdef HAVE_MALLINFO2
+static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
+{
+	tst_res(TINFO, "%s...", msg);
+#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
+	P2(arena);
+	P2(ordblks);
+	P2(smblks);
+	P2(hblks);
+	P2(hblkhd);
+	P2(usmblks);
+	P2(fsmblks);
+	P2(uordblks);
+	P2(fordblks);
+	P2(keepcost);
+}
+#endif
+
 #endif
diff --git a/testcases/kernel/syscalls/mallinfo2/.gitignore b/testcases/kernel/syscalls/mallinfo2/.gitignore
new file mode 100644
index 000000000..848f00130
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/.gitignore
@@ -0,0 +1 @@ 
+/mallinfo201
diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
new file mode 100644
index 000000000..044619fb8
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/Makefile
@@ -0,0 +1,8 @@ 
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) International Business Machines  Corp., 2001
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/mallinfo2/mallinfo201.c b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
new file mode 100644
index 000000000..0bc2520a0
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
@@ -0,0 +1,47 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [DESCRIPTION]
+ *
+ * Basic mallinfo2() test. Test the member of struct mallinfo2
+ * whether overflow when setting 2G size. mallinfo() is deprecated
+ * since the type used for the fields is too small. We should use
+ * mallinfo2() and it was added since glibc 2.33.
+ */
+
+#include "../mallinfo/mallinfo_common.h"
+#include "tst_safe_macros.h"
+
+#ifdef HAVE_MALLINFO2
+
+void test_mallinfo2(void)
+{
+	struct mallinfo2 info;
+	char *buf;
+	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
+
+	buf = malloc(size);
+	if (!buf) {
+		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
+		return;
+	}
+	info = mallinfo2();
+	if (info.hblkhd < size) {
+		print_mallinfo2("Test malloc 2G", &info);
+		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
+	}
+	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
+	free(buf);
+}
+
+static struct tst_test test = {
+	.test_all = test_mallinfo2,
+};
+
+#else
+TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
+#endif