public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] arm: add missing exports for asm functions required by get_user macro
@ 2014-09-13 19:39 Victor Kamensky
  2014-09-13 19:39 ` Victor Kamensky
  0 siblings, 1 reply; 4+ messages in thread
From: Victor Kamensky @ 2014-09-13 19:39 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

Patch following this cover latter fixes issue that you reported [1]
with '[PATCH V3] arm: fix get_user BE behavior for target variable with 
size of 8 bytes' patch:

ERROR: "__get_user_64t_4" [fs/ext4/ext4.ko] undefined!
ERROR: "__get_user_64t_4" [fs/cifs/cifs.ko] undefined!

The issue is that newly introduced get_user macro helper asm
functions were not exported. So in case of ext4 and cifs compiled
as kernel modules in BE image the issue got exposed.

While looking at this I realized that __get_user_8 helper
function added by other commit is not exported too. It would
affect LE image as well but fortunately currently there is 
no module that use get_user in such combination.

The proposed patch adds export for all recently added
asm helper functions used by get_user macro.

I've tested all combinations of target variable types 
(1, 2, 4, 8 bytes), value pointer type (1, 2, 4, 8 bytes), 
kernel/module, and BE/LE with test patch provided below.
It all looks good now abd it addresses reported issue.

Thanks,
Victor

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/287167.html

Appendix 1
----------

Patch that was used for testing of get_user macro export fix.
It is provided just for reference only.

>From d34a035769fae6a6dff9f407daa7f0e65bfe55a9 Mon Sep 17 00:00:00 2001
From: Victor Kamensky <victor.kamensky@linaro.org>
Date: Wed, 20 Aug 2014 19:31:28 -0700
Subject: [PATCH] arm: get_user tests

Test different combinations of value and pointers passed to
get_user macro.

Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
 arch/arm/kernel/Makefile         |   3 +
 arch/arm/kernel/get_user_test.h  |  54 ++++++++++++++
 arch/arm/kernel/get_user_test1.c |  77 ++++++++++++++++++++
 arch/arm/kernel/get_user_test2.c | 151 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 285 insertions(+)
 create mode 100644 arch/arm/kernel/get_user_test.h
 create mode 100644 arch/arm/kernel/get_user_test1.c
 create mode 100644 arch/arm/kernel/get_user_test2.c

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 38ddd9f..193e581 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -30,6 +30,9 @@ else
 obj-y		+= entry-armv.o
 endif
 
+obj-m		+= test-get_user.o
+test-get_user-objs += get_user_test1.o get_user_test2.o
+
 obj-$(CONFIG_OC_ETM)		+= etm.o
 obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 obj-$(CONFIG_ISA_DMA_API)	+= dma.o
diff --git a/arch/arm/kernel/get_user_test.h b/arch/arm/kernel/get_user_test.h
new file mode 100644
index 0000000..723f03a
--- /dev/null
+++ b/arch/arm/kernel/get_user_test.h
@@ -0,0 +1,54 @@
+typedef char	   tint8_t;
+typedef short	  tint16_t;
+typedef int	  tint32_t;
+typedef long long tint64_t;
+
+#define GUT_LOWER_TEST_DECLARE(vs, ps)			\
+	tint8_t gut_lower_v ## vs ## _p ## ps		\
+	(tint ## ps ## _t *ptr);
+
+GUT_LOWER_TEST_DECLARE(64, 8)
+GUT_LOWER_TEST_DECLARE(64, 16)
+GUT_LOWER_TEST_DECLARE(64, 32)
+GUT_LOWER_TEST_DECLARE(64, 64)
+
+GUT_LOWER_TEST_DECLARE(32, 8)
+GUT_LOWER_TEST_DECLARE(32, 16)
+GUT_LOWER_TEST_DECLARE(32, 32)
+GUT_LOWER_TEST_DECLARE(32, 64)
+
+GUT_LOWER_TEST_DECLARE(16, 8)
+GUT_LOWER_TEST_DECLARE(16, 16)
+GUT_LOWER_TEST_DECLARE(16, 32)
+GUT_LOWER_TEST_DECLARE(16, 64)
+
+GUT_LOWER_TEST_DECLARE(8, 8)
+GUT_LOWER_TEST_DECLARE(8, 16)
+GUT_LOWER_TEST_DECLARE(8, 32)
+GUT_LOWER_TEST_DECLARE(8, 64)
+
+
+#define GUT_UPPER_TEST_DECLARE(vs, ps)			\
+	tint8_t gut_upper_v ## vs ## _p ## ps		\
+	(tint ## ps ## _t *ptr);
+
+GUT_UPPER_TEST_DECLARE(64, 8)
+GUT_UPPER_TEST_DECLARE(64, 16)
+GUT_UPPER_TEST_DECLARE(64, 32)
+GUT_UPPER_TEST_DECLARE(64, 64)
+
+GUT_UPPER_TEST_DECLARE(32, 8)
+GUT_UPPER_TEST_DECLARE(32, 16)
+GUT_UPPER_TEST_DECLARE(32, 32)
+GUT_UPPER_TEST_DECLARE(32, 64)
+
+GUT_UPPER_TEST_DECLARE(16, 8)
+GUT_UPPER_TEST_DECLARE(16, 16)
+GUT_UPPER_TEST_DECLARE(16, 32)
+GUT_UPPER_TEST_DECLARE(16, 64)
+
+GUT_UPPER_TEST_DECLARE(8, 8)
+GUT_UPPER_TEST_DECLARE(8, 16)
+GUT_UPPER_TEST_DECLARE(8, 32)
+GUT_UPPER_TEST_DECLARE(8, 64)
+
diff --git a/arch/arm/kernel/get_user_test1.c b/arch/arm/kernel/get_user_test1.c
new file mode 100644
index 0000000..e34b168
--- /dev/null
+++ b/arch/arm/kernel/get_user_test1.c
@@ -0,0 +1,77 @@
+#include <asm/uaccess.h>
+
+#include "get_user_test.h"
+
+#define GUT_LOWER_TEST(vs, ps)				\
+	tint8_t gut_lower_v ## vs ## _p ## ps		\
+	(tint ## ps ## _t *ptr)				\
+{							\
+	tint ## vs ## _t value = 0;			\
+							\
+	mm_segment_t fs;				\
+	fs = get_fs();					\
+	set_fs(KERNEL_DS);				\
+							\
+	get_user(value, ptr);				\
+							\
+	set_fs(fs);					\
+	return 0xff & value;				\
+}							\
+
+GUT_LOWER_TEST(64, 8)
+GUT_LOWER_TEST(64, 16)
+GUT_LOWER_TEST(64, 32)
+GUT_LOWER_TEST(64, 64)
+
+GUT_LOWER_TEST(32, 8)
+GUT_LOWER_TEST(32, 16)
+GUT_LOWER_TEST(32, 32)
+GUT_LOWER_TEST(32, 64)
+
+GUT_LOWER_TEST(16, 8)
+GUT_LOWER_TEST(16, 16)
+GUT_LOWER_TEST(16, 32)
+GUT_LOWER_TEST(16, 64)
+
+GUT_LOWER_TEST(8, 8)
+GUT_LOWER_TEST(8, 16)
+GUT_LOWER_TEST(8, 32)
+GUT_LOWER_TEST(8, 64)
+
+
+#define GUT_UPPER_TEST(vs, ps)				\
+	tint8_t gut_upper_v ## vs ## _p ## ps		\
+	(tint ## ps ## _t *ptr)				\
+{							\
+	tint ## vs ## _t value = 0;			\
+							\
+	mm_segment_t fs;				\
+	fs = get_fs();					\
+	set_fs(KERNEL_DS);				\
+							\
+	get_user(value, ptr);				\
+							\
+	set_fs(fs);					\
+	return 0xff & (value >> ((vs > ps) ? (ps - 8) : (vs - 8)));	\
+}							\
+
+
+GUT_UPPER_TEST(64, 8)
+GUT_UPPER_TEST(64, 16)
+GUT_UPPER_TEST(64, 32)
+GUT_UPPER_TEST(64, 64)
+
+GUT_UPPER_TEST(32, 8)
+GUT_UPPER_TEST(32, 16)
+GUT_UPPER_TEST(32, 32)
+GUT_UPPER_TEST(32, 64)
+
+GUT_UPPER_TEST(16, 8)
+GUT_UPPER_TEST(16, 16)
+GUT_UPPER_TEST(16, 32)
+GUT_UPPER_TEST(16, 64)
+
+GUT_UPPER_TEST(8, 8)
+GUT_UPPER_TEST(8, 16)
+GUT_UPPER_TEST(8, 32)
+GUT_UPPER_TEST(8, 64)
diff --git a/arch/arm/kernel/get_user_test2.c b/arch/arm/kernel/get_user_test2.c
new file mode 100644
index 0000000..e483464
--- /dev/null
+++ b/arch/arm/kernel/get_user_test2.c
@@ -0,0 +1,151 @@
+#include <linux/module.h>
+#include "get_user_test.h"
+
+tint8_t gut_data8 = 0x12;
+tint16_t gut_data16 = 0x1234;
+tint32_t gut_data32 = 0x12345678;
+tint64_t gut_data64 = 0x1234567890abcdef;
+
+#define GUT_RUN_LOWER_TEST(vs, ps, expect)				\
+	void gut_run_lower_v ## vs ## _p ## ps				\
+	(void)								\
+{									\
+	tint8_t ret = gut_lower_v ## vs ## _p ## ps (&gut_data ## ps);	\
+	if (ret == expect) {						\
+		printk("GUT_TEST: lower v" #vs "_" #ps ": PASS\n");	\
+	} else {							\
+		printk("GUT_TEST: lower v" #vs "_" #ps			\
+		       ": FAILED: expected 0x%x got 0x%x\n",		\
+		       expect, ret);					\
+	}								\
+}
+
+
+GUT_RUN_LOWER_TEST(64, 8, 0x12)
+GUT_RUN_LOWER_TEST(64, 16, 0x34)
+GUT_RUN_LOWER_TEST(64, 32, 0x78)
+GUT_RUN_LOWER_TEST(64, 64, 0xef)
+
+GUT_RUN_LOWER_TEST(32, 8, 0x12)
+GUT_RUN_LOWER_TEST(32, 16, 0x34)
+GUT_RUN_LOWER_TEST(32, 32, 0x78)
+GUT_RUN_LOWER_TEST(32, 64, 0xef)
+
+GUT_RUN_LOWER_TEST(16, 8, 0x12)
+GUT_RUN_LOWER_TEST(16, 16, 0x34)
+GUT_RUN_LOWER_TEST(16, 32, 0x78)
+GUT_RUN_LOWER_TEST(16, 64, 0xef)
+
+GUT_RUN_LOWER_TEST(8, 8, 0x12)
+GUT_RUN_LOWER_TEST(8, 16, 0x34)
+GUT_RUN_LOWER_TEST(8, 32, 0x78)
+GUT_RUN_LOWER_TEST(8, 64, 0xef)
+
+#define GUT_RUN_UPPER_TEST(vs, ps, expect)				\
+	void gut_run_upper_v ## vs ## _p ## ps				\
+	(void)								\
+{									\
+	tint8_t ret = gut_upper_v ## vs ## _p ## ps (&gut_data ## ps);	\
+	if (ret == expect) {						\
+		printk("GUT_TEST: upper v" #vs "_" #ps ": PASS\n");	\
+	} else {							\
+		printk("GUT_TEST: upper v" #vs "_" #ps			\
+		       ": FAILED: expected 0x%x got 0x%x\n",		\
+		       expect, ret);					\
+	}								\
+}
+
+GUT_RUN_UPPER_TEST(64, 8, 0x12)
+GUT_RUN_UPPER_TEST(64, 16, 0x12)
+GUT_RUN_UPPER_TEST(64, 32, 0x12)
+GUT_RUN_UPPER_TEST(64, 64, 0x12)
+
+GUT_RUN_UPPER_TEST(32, 8, 0x12)
+GUT_RUN_UPPER_TEST(32, 16, 0x12)
+GUT_RUN_UPPER_TEST(32, 32, 0x12)
+GUT_RUN_UPPER_TEST(32, 64, 0x90)
+
+GUT_RUN_UPPER_TEST(16, 8, 0x12)
+GUT_RUN_UPPER_TEST(16, 16, 0x12)
+GUT_RUN_UPPER_TEST(16, 32, 0x56)
+GUT_RUN_UPPER_TEST(16, 64, 0xcd)
+
+GUT_RUN_UPPER_TEST(8, 8, 0x12)
+GUT_RUN_UPPER_TEST(8, 16, 0x34)
+GUT_RUN_UPPER_TEST(8, 32, 0x78)
+GUT_RUN_UPPER_TEST(8, 64, 0xef)
+
+
+#define GUT_RUN_LOWER_TEST_CALL(vs, ps) gut_run_lower_v ## vs ## _p ## ps ()
+#define GUT_RUN_UPPER_TEST_CALL(vs, ps) gut_run_upper_v ## vs ## _p ## ps ()
+
+static int
+run_all_tests (void)
+{
+	printk("GUT_TEST: start\n");
+
+	GUT_RUN_LOWER_TEST_CALL(64, 8);
+	GUT_RUN_LOWER_TEST_CALL(64, 16);
+	GUT_RUN_LOWER_TEST_CALL(64, 32);
+	GUT_RUN_LOWER_TEST_CALL(64, 64);
+
+	GUT_RUN_LOWER_TEST_CALL(32, 8);
+	GUT_RUN_LOWER_TEST_CALL(32, 16);
+	GUT_RUN_LOWER_TEST_CALL(32, 32);
+	GUT_RUN_LOWER_TEST_CALL(32, 64);
+
+	GUT_RUN_LOWER_TEST_CALL(16, 8);
+	GUT_RUN_LOWER_TEST_CALL(16, 16);
+	GUT_RUN_LOWER_TEST_CALL(16, 32);
+	GUT_RUN_LOWER_TEST_CALL(16, 64);
+
+	GUT_RUN_LOWER_TEST_CALL(8, 8);
+	GUT_RUN_LOWER_TEST_CALL(8, 16);
+	GUT_RUN_LOWER_TEST_CALL(8, 32);
+	GUT_RUN_LOWER_TEST_CALL(8, 64);
+
+
+	GUT_RUN_UPPER_TEST_CALL(64, 8);
+	GUT_RUN_UPPER_TEST_CALL(64, 16);
+	GUT_RUN_UPPER_TEST_CALL(64, 32);
+	GUT_RUN_UPPER_TEST_CALL(64, 64);
+
+	GUT_RUN_UPPER_TEST_CALL(32, 8);
+	GUT_RUN_UPPER_TEST_CALL(32, 16);
+	GUT_RUN_UPPER_TEST_CALL(32, 32);
+	GUT_RUN_UPPER_TEST_CALL(32, 64);
+
+	GUT_RUN_UPPER_TEST_CALL(16, 8);
+	GUT_RUN_UPPER_TEST_CALL(16, 16);
+	GUT_RUN_UPPER_TEST_CALL(16, 32);
+	GUT_RUN_UPPER_TEST_CALL(16, 64);
+
+	GUT_RUN_UPPER_TEST_CALL(8, 8);
+	GUT_RUN_UPPER_TEST_CALL(8, 16);
+	GUT_RUN_UPPER_TEST_CALL(8, 32);
+	GUT_RUN_UPPER_TEST_CALL(8, 64);
+
+	printk("GUT_TEST: finished\n");
+	return 0;
+}
+
+/*
+ * Module setup
+ */
+
+#ifdef MODULE
+
+static void
+test_exit(void)
+{
+}
+
+module_init(run_all_tests)
+module_exit(test_exit)
+MODULE_LICENSE("GPL");
+
+#else /* !MODULE */
+
+late_initcall(run_all_tests);
+
+#endif
-- 
1.8.1.4


Victor Kamensky (1):
  arm: add missing exports for asm functions required by get_user macro

 arch/arm/kernel/armksyms.c | 8 ++++++++
 1 file changed, 8 insertions(+)

-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] arm: add missing exports for asm functions required by get_user macro
  2014-09-13 19:39 [PATCH] arm: add missing exports for asm functions required by get_user macro Victor Kamensky
@ 2014-09-13 19:39 ` Victor Kamensky
  2014-09-13 22:00   ` Russell King - ARM Linux
  0 siblings, 1 reply; 4+ messages in thread
From: Victor Kamensky @ 2014-09-13 19:39 UTC (permalink / raw)
  To: linux-arm-kernel

Previous commits that dealt with get_user for 64bit type missed to
export proper functions, so if get_user macro with particular target/value
types are used by kernel module modpost would produce 'undefined!' error.
Solution is to export all required functions.

Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
 arch/arm/kernel/armksyms.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/kernel/armksyms.c b/arch/arm/kernel/armksyms.c
index f7b450f..a88671c 100644
--- a/arch/arm/kernel/armksyms.c
+++ b/arch/arm/kernel/armksyms.c
@@ -98,6 +98,14 @@ EXPORT_SYMBOL(__clear_user);
 EXPORT_SYMBOL(__get_user_1);
 EXPORT_SYMBOL(__get_user_2);
 EXPORT_SYMBOL(__get_user_4);
+EXPORT_SYMBOL(__get_user_8);
+
+#ifdef __ARMEB__
+EXPORT_SYMBOL(__get_user_64t_1);
+EXPORT_SYMBOL(__get_user_64t_2);
+EXPORT_SYMBOL(__get_user_64t_4);
+EXPORT_SYMBOL(__get_user_32t_8);
+#endif
 
 EXPORT_SYMBOL(__put_user_1);
 EXPORT_SYMBOL(__put_user_2);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] arm: add missing exports for asm functions required by get_user macro
  2014-09-13 19:39 ` Victor Kamensky
@ 2014-09-13 22:00   ` Russell King - ARM Linux
  2014-09-13 22:34     ` Victor Kamensky
  0 siblings, 1 reply; 4+ messages in thread
From: Russell King - ARM Linux @ 2014-09-13 22:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Sep 13, 2014 at 12:39:21PM -0700, Victor Kamensky wrote:
> Previous commits that dealt with get_user for 64bit type missed to
> export proper functions, so if get_user macro with particular target/value
> types are used by kernel module modpost would produce 'undefined!' error.
> Solution is to export all required functions.
> 
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>

Patch looks good on its own, please send to the patch system.

I can either apply it as a follow on patch, or I can merge it into your
original patch.  Which would you prefer?

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] arm: add missing exports for asm functions required by get_user macro
  2014-09-13 22:00   ` Russell King - ARM Linux
@ 2014-09-13 22:34     ` Victor Kamensky
  0 siblings, 0 replies; 4+ messages in thread
From: Victor Kamensky @ 2014-09-13 22:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 September 2014 15:00, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Sep 13, 2014 at 12:39:21PM -0700, Victor Kamensky wrote:
>> Previous commits that dealt with get_user for 64bit type missed to
>> export proper functions, so if get_user macro with particular target/value
>> types are used by kernel module modpost would produce 'undefined!' error.
>> Solution is to export all required functions.
>>
>> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
>
> Patch looks good on its own, please send to the patch system.

Submitted as
   http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8151/1

> I can either apply it as a follow on patch, or I can merge it into your
> original patch.  Which would you prefer?

Either way is fine with me, whatever easier for you.

Thanks,
Victor

> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-09-13 22:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-13 19:39 [PATCH] arm: add missing exports for asm functions required by get_user macro Victor Kamensky
2014-09-13 19:39 ` Victor Kamensky
2014-09-13 22:00   ` Russell King - ARM Linux
2014-09-13 22:34     ` Victor Kamensky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox