diff mbox

[v2,06/20] randstruct: Whitelist UNIXCB cast

Message ID 1495829844-69341-7-git-send-email-keescook@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kees Cook May 26, 2017, 8:17 p.m. UTC
This is another false positive in bad cast detection:

net/unix/af_unix.c: In function ‘unix_skb_scm_eq’:
net/unix/af_unix.c:1621:31: note: found mismatched rhs struct pointer types: ‘struct unix_skb_parms’ and ‘char’

  const struct unix_skb_parms *u = &UNIXCB(skb);
                               ^

UNIXCB is:

	#define UNIXCB(skb)     (*(struct unix_skb_parms *)&((skb)->cb))

And ->cb is:

	char                    cb[48] __aligned(8);

This is a rather crazy cast, but appears to be safe in the face of
randomization, so whitelist it in the plugin.

Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Kees Cook May 26, 2017, 8:20 p.m. UTC | #1
On Fri, May 26, 2017 at 1:17 PM, Kees Cook <keescook@chromium.org> wrote:
> This is another false positive in bad cast detection:
>
> net/unix/af_unix.c: In function ‘unix_skb_scm_eq’:
> net/unix/af_unix.c:1621:31: note: found mismatched rhs struct pointer types: ‘struct unix_skb_parms’ and ‘char’
>
>   const struct unix_skb_parms *u = &UNIXCB(skb);
>                                ^
>
> UNIXCB is:
>
>         #define UNIXCB(skb)     (*(struct unix_skb_parms *)&((skb)->cb))
>
> And ->cb is:
>
>         char                    cb[48] __aligned(8);
>
> This is a rather crazy cast, but appears to be safe in the face of
> randomization, so whitelist it in the plugin.
>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: Signed-off-by: David S. Miller <davem@davemloft.net>

Argh, paste-o. I was adding git history folks here to Cc, pardon the
glitch. This should be Cc: and absolutely not a S-o-B from Dave.

-Kees

> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  scripts/gcc-plugins/randomize_layout_plugin.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
> index e126ac7874af..bf110915a5aa 100644
> --- a/scripts/gcc-plugins/randomize_layout_plugin.c
> +++ b/scripts/gcc-plugins/randomize_layout_plugin.c
> @@ -43,6 +43,8 @@ struct whitelist_entry {
>  };
>
>  static const struct whitelist_entry whitelist[] = {
> +       /* unix_skb_parms via UNIXCB() buffer */
> +       { "net/unix/af_unix.c", "unix_skb_parms", "char" },
>         /* walk struct security_hook_heads as an array of struct list_head */
>         { "security/security.c", "list_head", "security_hook_heads" },
>         { }
> --
> 2.7.4
>
Christoph Hellwig May 28, 2017, 7:56 a.m. UTC | #2
On Fri, May 26, 2017 at 01:17:10PM -0700, Kees Cook wrote:
> This is another false positive in bad cast detection:
> 
> net/unix/af_unix.c: In function ‘unix_skb_scm_eq’:
> net/unix/af_unix.c:1621:31: note: found mismatched rhs struct pointer types: ‘struct unix_skb_parms’ and ‘char’
> 
>   const struct unix_skb_parms *u = &UNIXCB(skb);
>                                ^
> 
> UNIXCB is:
> 
> 	#define UNIXCB(skb)     (*(struct unix_skb_parms *)&((skb)->cb))
> 
> And ->cb is:
> 
> 	char                    cb[48] __aligned(8);
> 
> This is a rather crazy cast, but appears to be safe in the face of
> randomization, so whitelist it in the plugin.

We have a lot of network protocol that use the ->cb area, which makes me
wonder why this one would be so special.

It seems like everyone is just using a plain cast to a pointer without
doing the address taking trick that doesn't make sense for arrays
anyway.

Maybe we just need to fix up the af_unix code to work the same as all
other protocols?
diff mbox

Patch

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index e126ac7874af..bf110915a5aa 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -43,6 +43,8 @@  struct whitelist_entry {
 };
 
 static const struct whitelist_entry whitelist[] = {
+	/* unix_skb_parms via UNIXCB() buffer */
+	{ "net/unix/af_unix.c", "unix_skb_parms", "char" },
 	/* walk struct security_hook_heads as an array of struct list_head */
 	{ "security/security.c", "list_head", "security_hook_heads" },
 	{ }