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

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

[Eric Dumazet]

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",
+	       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)

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

[Joe Perches]

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?

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

[Eric Dumazet]

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.

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

[Ben Hutchings]

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)


-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

[David Miller]

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.

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

[Eric Dumazet]

On Sat, 2013-05-18 at 02:11 +0100, Ben Hutchings wrote:

> Not "%pK"?
> 

Yes, thanks for the suggestion.

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

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

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

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
Cc: Daniel Borkmann <dborkman@redhat.com>
---
v2: use %pK as Ben suggestion

 include/linux/filter.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index c050dcc..56a6b7f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -58,10 +58,10 @@ 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",
+	pr_err("flen=%u proglen=%u pass=%u image=%pK\n",
 	       flen, proglen, pass, image);
 	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)

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

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 17 May 2013 19:57:37 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Do not leak starting address of BPF JIT code for non root users,
> as it might help intruders to perform an attack.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.