diff mbox

[net-next] filter: do not output bpf image address for security reason

Message ID 1368833499.3301.126.camel@edumazet-glaptop
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet May 17, 2013, 11:31 p.m. UTC
From: Eric Dumazet <edumazet@google.com>

Do not leak starting address of BPF JIT code, as it might help
intruders to perform an attack.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Daniel Borkmann <dborkman@redhat.com>
---
 include/linux/filter.h |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Joe Perches May 17, 2013, 11:42 p.m. UTC | #1
On Fri, 2013-05-17 at 16:31 -0700, Eric Dumazet wrote:
> Do not leak starting address of BPF JIT code, as it might help
> intruders to perform an attack.
[]
> diff --git a/include/linux/filter.h b/include/linux/filter.h
[]
> @@ -58,10 +58,11 @@ extern void bpf_jit_free(struct sk_filter *fp);
>  static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen,
>  				u32 pass, void *image)
>  {
> -	pr_err("flen=%u proglen=%u pass=%u image=%p\n",
> -	       flen, proglen, pass, image);
> +	/* Do not output address (image) for security reason */
> +	pr_err("flen=%u proglen=%u pass=%u image=10\n",
> +	       flen, proglen, pass);

[]

Are stable equivalents for versions before commit 79617801ea0
necessary?

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet May 17, 2013, 11:48 p.m. UTC | #2
On Fri, 2013-05-17 at 16:42 -0700, Joe Perches wrote:

> Are stable equivalents for versions before commit 79617801ea0
> necessary?
> 

I do not think so.

In order to get these messages printed, the admin had to specifically do

echo 2 >/proc/sys/net/core/bpf_jit_enable

And quite frankly I doubt anybody would need to do such thing, but
netdev guys writing/patching BPF JIT

And even with these messages printed, you need some bug in the kernel
allowing an exploit.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings May 18, 2013, 1:11 a.m. UTC | #3
On Fri, 2013-05-17 at 16:31 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Do not leak starting address of BPF JIT code, as it might help
> intruders to perform an attack.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> ---
>  include/linux/filter.h |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index c050dcc..08cda1c 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -58,10 +58,11 @@ extern void bpf_jit_free(struct sk_filter *fp);
>  static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen,
>  				u32 pass, void *image)
>  {
> -	pr_err("flen=%u proglen=%u pass=%u image=%p\n",
> -	       flen, proglen, pass, image);
> +	/* Do not output address (image) for security reason */
> +	pr_err("flen=%u proglen=%u pass=%u image=10\n",

Not "%pK"?

Ben.

> +	       flen, proglen, pass);
>  	if (image)
> -		print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_ADDRESS,
> +		print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_OFFSET,
>  			       16, 1, image, proglen, false);
>  }
>  #define SK_RUN_FILTER(FILTER, SKB) (*FILTER->bpf_func)(SKB, FILTER->insns)
David Miller May 18, 2013, 1:27 a.m. UTC | #4
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Sat, 18 May 2013 02:11:18 +0100

> On Fri, 2013-05-17 at 16:31 -0700, Eric Dumazet wrote:
>> @@ -58,10 +58,11 @@ extern void bpf_jit_free(struct sk_filter *fp);
>>  static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen,
>>  				u32 pass, void *image)
>>  {
>> -	pr_err("flen=%u proglen=%u pass=%u image=%p\n",
>> -	       flen, proglen, pass, image);
>> +	/* Do not output address (image) for security reason */
>> +	pr_err("flen=%u proglen=%u pass=%u image=10\n",
> 
> Not "%pK"?

Yes, that's the acceptable way to deal with this issue.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet May 18, 2013, 2:55 a.m. UTC | #5
On Sat, 2013-05-18 at 02:11 +0100, Ben Hutchings wrote:

> Not "%pK"?
> 

Yes, thanks for the suggestion.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/filter.h b/include/linux/filter.h
index c050dcc..08cda1c 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -58,10 +58,11 @@  extern void bpf_jit_free(struct sk_filter *fp);
 static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen,
 				u32 pass, void *image)
 {
-	pr_err("flen=%u proglen=%u pass=%u image=%p\n",
-	       flen, proglen, pass, image);
+	/* Do not output address (image) for security reason */
+	pr_err("flen=%u proglen=%u pass=%u image=10\n",
+	       flen, proglen, pass);
 	if (image)
-		print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_ADDRESS,
+		print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_OFFSET,
 			       16, 1, image, proglen, false);
 }
 #define SK_RUN_FILTER(FILTER, SKB) (*FILTER->bpf_func)(SKB, FILTER->insns)