sgi-xp: use designated initializers

PATCH Superseded Kees Cook 2 participants 6 messages Dec 17, 2016 – May 8, 2017 Char and misc drivers

Share & tools

Kees Cook

author

2016-12-17 01:01 UTC #0 lore raw

PATCH 1/1 sgi-xp: use designated initializers

Prepare to mark sensitive kernel structures for randomization by making sure they're using designated initializers. These were identified during allyesconfig builds of x86, arm, and arm64, with most initializer fixes extracted from grsecurity.

Signed-off-by Kees Cook <[email protected]>

---
 drivers/misc/sgi-xp/xp_main.c | 59 ++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 20 deletions(-)

drivers/misc/sgi-xp/xp_main.c +39 ~~-20~~

index 01be66d02ca8..bb47f9d9b68a 100644
@@ -71,20 +71,44 @@ EXPORT_SYMBOL_GPL(xpc_registrations);
 /*
  * Initialize the XPC interface to indicate that XPC isn't loaded.
  */
-static enum xp_retval
-xpc_notloaded(void)
+static void xpc_notloaded_connect(int ch_number)
+{ }
+
+static void xpc_notloaded_disconnect(int ch_number)
+{ }
+
+static enum xp_retval xpc_notloaded_send(short partid, int ch_number,
+					 u32 flags, void *payload,
+					 u16 payload_size)
+{
+	return xpNotLoaded;
+}
+
+static enum xp_retval xpc_notloaded_send_notify(short partid, int ch_number,
+						u32 flags, void *payload,
+						u16 payload_size,
+						xpc_notify_func func,
+						void *key)
+{
+	return xpNotLoaded;
+}
+
+static void xpc_notloaded_received(short partid, int ch_number, void *payload)
+{ }
+
+static enum xp_retval xpc_notloaded_partid_to_nasids(short partid,
+						     void *nasid_mask)
 {
 	return xpNotLoaded;
 }
 
 struct xpc_interface xpc_interface = {
-	(void (*)(int))xpc_notloaded,
-	(void (*)(int))xpc_notloaded,
-	(enum xp_retval(*)(short, int, u32, void *, u16))xpc_notloaded,
-	(enum xp_retval(*)(short, int, u32, void *, u16, xpc_notify_func,
-			   void *))xpc_notloaded,
-	(void (*)(short, int, void *))xpc_notloaded,
-	(enum xp_retval(*)(short, void *))xpc_notloaded
+	.connect = xpc_notloaded_connect,
+	.disconnect = xpc_notloaded_disconnect,
+	.send = xpc_notloaded_send,
+	.send_notify = xpc_notloaded_send_notify,
+	.received = xpc_notloaded_received,
+	.partid_to_nasids = xpc_notloaded_partid_to_nasids
 };
 EXPORT_SYMBOL_GPL(xpc_interface);

@@ -115,17 +139,12 @@ EXPORT_SYMBOL_GPL(xpc_set_interface);
 void
 xpc_clear_interface(void)
 {
-	xpc_interface.connect = (void (*)(int))xpc_notloaded;
-	xpc_interface.disconnect = (void (*)(int))xpc_notloaded;
-	xpc_interface.send = (enum xp_retval(*)(short, int, u32, void *, u16))
-	    xpc_notloaded;
-	xpc_interface.send_notify = (enum xp_retval(*)(short, int, u32, void *,
-						       u16, xpc_notify_func,
-						       void *))xpc_notloaded;
-	xpc_interface.received = (void (*)(short, int, void *))
-	    xpc_notloaded;
-	xpc_interface.partid_to_nasids = (enum xp_retval(*)(short, void *))
-	    xpc_notloaded;
+	xpc_interface.connect = xpc_notloaded_connect;
+	xpc_interface.disconnect = xpc_notloaded_disconnect;
+	xpc_interface.send = xpc_notloaded_send;
+	xpc_interface.send_notify = xpc_notloaded_send_notify;
+	xpc_interface.received = xpc_notloaded_received;
+	xpc_interface.partid_to_nasids = xpc_notloaded_partid_to_nasids;
 }
 EXPORT_SYMBOL_GPL(xpc_clear_interface);

Signature · 6 lines

2.7.4


-- 
Kees Cook
Nexus Security

Robin Holt

2016-12-21 16:24 UTC #1 lore raw

Quoted 4 lines · Kees Cook

Prepare to mark sensitive kernel structures for randomization by making sure they're using designated initializers. These were identified during allyesconfig builds of x86, arm, and arm64, with most initializer fixes extracted from grsecurity.

I guess I don't understand the context enough here to give you a Signed-off-by. Can you give us more background on this randomization?

Quoted 1 line

From what I see in the code here, I can see you are providing

equivalent functionality and I would give it a signed-off-by, but I am not sure this randomization of which you speak is not going to cause problems for XP, XPC, XPNET, and XPMEM (out of tree GPL kernel module).

Robin

Kees Cook

author

2017-01-03 23:19 UTC #2 lore raw

Quoted 8 lines · Robin Holt

Quoted 4 lines · Kees Cook
Prepare to mark sensitive kernel structures for randomization by making sure they're using designated initializers. These were identified during allyesconfig builds of x86, arm, and arm64, with most initializer fixes extracted from grsecurity.
I guess I don't understand the context enough here to give you a Signed-off-by. Can you give us more background on this randomization?

Sure thing! The randomization is on the order of function pointers in all-pointer structures (like struct xpc_interface). As long as the memory containing the structure isn't shared externally, this randomization should have no operational effect. The reason explicit no-op functions were added was to avoid ugly casts, etc.

Quoted 5 lines

From what I see in the code here, I can see you are providing equivalent functionality and I would give it a signed-off-by, but I am not sure this randomization of which you speak is not going to cause problems for XP, XPC, XPNET, and XPMEM (out of tree GPL kernel module).

Ah, hm, does this module share the structure without being built against the kernel? (If built with the kernel, the randomization plugin will keep things in the right order.)

-Kees

Signature · 2 lines

Kees Cook
Nexus Security

Kees Cook

author

2017-03-29 20:48 UTC #3 lore raw

Quoted 7 lines · Kees Cook

Quoted 6 lines · Robin Holt
Quoted 4 lines · Kees Cook
Prepare to mark sensitive kernel structures for randomization by making sure they're using designated initializers. These were identified during allyesconfig builds of x86, arm, and arm64, with most initializer fixes extracted from grsecurity.

Quoted 8 lines

Quoted 2 lines
I guess I don't understand the context enough here to give you a Signed-off-by. Can you give us more background on this randomization?
Sure thing! The randomization is on the order of function pointers in all-pointer structures (like struct xpc_interface). As long as the memory containing the structure isn't shared externally, this randomization should have no operational effect. The reason explicit no-op functions were added was to avoid ugly casts, etc.

Friendly ping ... any chance this can land in -next soon?

Quoted 9 lines

Quoted 5 lines
From what I see in the code here, I can see you are providing equivalent functionality and I would give it a signed-off-by, but I am not sure this randomization of which you speak is not going to cause problems for XP, XPC, XPNET, and XPMEM (out of tree GPL kernel module).
Ah, hm, does this module share the structure without being built against the kernel? (If built with the kernel, the randomization plugin will keep things in the right order.)

Regardless of your answer, this randomization can be turned off. Switching to designated initializers here is mainly just a clean up.

Thanks!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Kees Cook

author

2017-05-06 07:16 UTC #4 lore raw

Quoted 25 lines · Kees Cook

Quoted 7 lines · Kees Cook
Quoted 6 lines · Robin Holt
Quoted 4 lines · Kees Cook
Prepare to mark sensitive kernel structures for randomization by making sure they're using designated initializers. These were identified during allyesconfig builds of x86, arm, and arm64, with most initializer fixes extracted from grsecurity.
Quoted 8 lines
Quoted 2 lines
I guess I don't understand the context enough here to give you a Signed-off-by. Can you give us more background on this randomization?
Sure thing! The randomization is on the order of function pointers in all-pointer structures (like struct xpc_interface). As long as the memory containing the structure isn't shared externally, this randomization should have no operational effect. The reason explicit no-op functions were added was to avoid ugly casts, etc.
Friendly ping ... any chance this can land in -next soon?
Quoted 6 lines
Quoted 5 lines
From what I see in the code here, I can see you are providing equivalent functionality and I would give it a signed-off-by, but I am not sure this randomization of which you speak is not going to cause problems for XP, XPC, XPNET, and XPMEM (out of tree GPL kernel module).

Quoted 6 lines

Quoted 3 lines
Ah, hm, does this module share the structure without being built against the kernel? (If built with the kernel, the randomization plugin will keep things in the right order.)
Regardless of your answer, this randomization can be turned off. Switching to designated initializers here is mainly just a clean up.

Any thoughts on this? I'd love to get this landed, and I'm happy to carry the patch myself, if it helps. Anyone able to Ack?

Thanks!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Robin Holt

Acked-by

2017-05-08 17:32 UTC #5 lore raw

Acked-by Robin Holt <[email protected]>

Robin Holt