[SRU,Xenial] UBUNTU: SAUCE: Add missing hunks from "bpf: fix branch pruning logic"

Message ID 20180413122108.871-1-seth.forshee@canonical.com
State New
Headers show
Series
  • [SRU,Xenial] UBUNTU: SAUCE: Add missing hunks from "bpf: fix branch pruning logic"
Related show

Commit Message

Seth Forshee April 13, 2018, 12:21 p.m.
BugLink: http://bugs.launchpad.net/bugs/1763454

At the time this commit was backported some of the code it
modifies was not present. When the code was later introduced from
upstream stable it did not get the changes from this commit.
Backport those changes now.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 kernel/bpf/verifier.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Colin Ian King April 13, 2018, 1:44 p.m. | #1
On 13/04/18 13:21, Seth Forshee wrote:
> BugLink: http://bugs.launchpad.net/bugs/1763454
> 
> At the time this commit was backported some of the code it
> modifies was not present. When the code was later introduced from
> upstream stable it did not get the changes from this commit.
> Backport those changes now.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  kernel/bpf/verifier.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8a40719c6ae5..c2b2743cec83 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2005,6 +2005,7 @@ process_bpf_exit:
>  					return err;
>  
>  				insn_idx++;
> +				env->insn_aux_data[insn_idx].seen = true;
>  			} else {
>  				verbose("invalid BPF_LD mode\n");
>  				return -EINVAL;
> @@ -2161,6 +2162,7 @@ static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
>  				u32 off, u32 cnt)
>  {
>  	struct bpf_insn_aux_data *new_data, *old_data = env->insn_aux_data;
> +	int i;
>  
>  	if (cnt == 1)
>  		return 0;
> @@ -2170,6 +2172,8 @@ static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
>  	memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
>  	memcpy(new_data + off + cnt - 1, old_data + off,
>  	       sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
> +	for (i = off; i < off + cnt - 1; i++)
> +		new_data[i].seen = true;
>  	env->insn_aux_data = new_data;
>  	vfree(old_data);
>  	return 0;
> 

This has positive test results, and addresses the backport issue, so..

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader April 13, 2018, 2:14 p.m. | #2
On 13.04.2018 14:21, Seth Forshee wrote:
> BugLink: http://bugs.launchpad.net/bugs/1763454
> 
> At the time this commit was backported some of the code it
> modifies was not present. When the code was later introduced from
> upstream stable it did not get the changes from this commit.
> Backport those changes now.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  kernel/bpf/verifier.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8a40719c6ae5..c2b2743cec83 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2005,6 +2005,7 @@ process_bpf_exit:
>  					return err;
>  
>  				insn_idx++;
> +				env->insn_aux_data[insn_idx].seen = true;
>  			} else {
>  				verbose("invalid BPF_LD mode\n");
>  				return -EINVAL;
> @@ -2161,6 +2162,7 @@ static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
>  				u32 off, u32 cnt)
>  {
>  	struct bpf_insn_aux_data *new_data, *old_data = env->insn_aux_data;
> +	int i;
>  
>  	if (cnt == 1)
>  		return 0;
> @@ -2170,6 +2172,8 @@ static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
>  	memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
>  	memcpy(new_data + off + cnt - 1, old_data + off,
>  	       sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
> +	for (i = off; i < off + cnt - 1; i++)
> +		new_data[i].seen = true;
>  	env->insn_aux_data = new_data;
>  	vfree(old_data);
>  	return 0;
>
Juerg Haefliger April 16, 2018, 7:24 a.m. | #3
On 04/13/2018 02:21 PM, Seth Forshee wrote:
> BugLink: http://bugs.launchpad.net/bugs/1763454
> 
> At the time this commit was backported some of the code it
> modifies was not present. When the code was later introduced from
> upstream stable it did not get the changes from this commit.
> Backport those changes now.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Shouldn't this have the CVE identifier or did I misunderstand that this
is part of a backported CVE fix?

...Juerg


> ---
>  kernel/bpf/verifier.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8a40719c6ae5..c2b2743cec83 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2005,6 +2005,7 @@ process_bpf_exit:
>  					return err;
>  
>  				insn_idx++;
> +				env->insn_aux_data[insn_idx].seen = true;
>  			} else {
>  				verbose("invalid BPF_LD mode\n");
>  				return -EINVAL;
> @@ -2161,6 +2162,7 @@ static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
>  				u32 off, u32 cnt)
>  {
>  	struct bpf_insn_aux_data *new_data, *old_data = env->insn_aux_data;
> +	int i;
>  
>  	if (cnt == 1)
>  		return 0;
> @@ -2170,6 +2172,8 @@ static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
>  	memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
>  	memcpy(new_data + off + cnt - 1, old_data + off,
>  	       sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
> +	for (i = off; i < off + cnt - 1; i++)
> +		new_data[i].seen = true;
>  	env->insn_aux_data = new_data;
>  	vfree(old_data);
>  	return 0;
>
Seth Forshee April 16, 2018, 12:42 p.m. | #4
On Mon, Apr 16, 2018 at 09:24:08AM +0200, Juerg Haefliger wrote:
> On 04/13/2018 02:21 PM, Seth Forshee wrote:
> > BugLink: http://bugs.launchpad.net/bugs/1763454
> > 
> > At the time this commit was backported some of the code it
> > modifies was not present. When the code was later introduced from
> > upstream stable it did not get the changes from this commit.
> > Backport those changes now.
> > 
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> Shouldn't this have the CVE identifier or did I misunderstand that this
> is part of a backported CVE fix?

Maybe ... it is part of the CVE fix, though I don't believe the
omission leaves the kernel vulnerable to the CVE. I guess maybe it
should have the id though.

Want me to resend or just want to add it when applying? The CVE id is
CVE-2017-17862.
Juerg Haefliger April 16, 2018, 12:51 p.m. | #5
On 04/16/2018 02:42 PM, Seth Forshee wrote:
> On Mon, Apr 16, 2018 at 09:24:08AM +0200, Juerg Haefliger wrote:
>> On 04/13/2018 02:21 PM, Seth Forshee wrote:
>>> BugLink: http://bugs.launchpad.net/bugs/1763454
>>>
>>> At the time this commit was backported some of the code it
>>> modifies was not present. When the code was later introduced from
>>> upstream stable it did not get the changes from this commit.
>>> Backport those changes now.
>>>
>>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>>
>> Shouldn't this have the CVE identifier or did I misunderstand that this
>> is part of a backported CVE fix?
> 
> Maybe ... it is part of the CVE fix, though I don't believe the
> omission leaves the kernel vulnerable to the CVE. I guess maybe it
> should have the id though.

Yeah I was just wondering if it helps to clarify that the commits belong
together.

> Want me to resend or just want to add it when applying? The CVE id is
> CVE-2017-17862.

Add when applying works for me. Whatever is easiest.

...Juerg
Seth Forshee April 17, 2018, 3:21 p.m. | #6
On Fri, Apr 13, 2018 at 07:21:08AM -0500, Seth Forshee wrote:
> BugLink: http://bugs.launchpad.net/bugs/1763454
> 
> At the time this commit was backported some of the code it
> modifies was not present. When the code was later introduced from
> upstream stable it did not get the changes from this commit.
> Backport those changes now.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Another issue was identified, I will send an updated patch.

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8a40719c6ae5..c2b2743cec83 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2005,6 +2005,7 @@  process_bpf_exit:
 					return err;
 
 				insn_idx++;
+				env->insn_aux_data[insn_idx].seen = true;
 			} else {
 				verbose("invalid BPF_LD mode\n");
 				return -EINVAL;
@@ -2161,6 +2162,7 @@  static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
 				u32 off, u32 cnt)
 {
 	struct bpf_insn_aux_data *new_data, *old_data = env->insn_aux_data;
+	int i;
 
 	if (cnt == 1)
 		return 0;
@@ -2170,6 +2172,8 @@  static int adjust_insn_aux_data(struct verifier_env *env, u32 prog_len,
 	memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
 	memcpy(new_data + off + cnt - 1, old_data + off,
 	       sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
+	for (i = off; i < off + cnt - 1; i++)
+		new_data[i].seen = true;
 	env->insn_aux_data = new_data;
 	vfree(old_data);
 	return 0;