| Message ID | 20210317185320.1561608-2-cascardo@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/2] neighbour: allow referenced neighbours to be removed | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 4 maintainers not CCed: jdike@akamai.com liuhangbin@gmail.com nikolay@cumulusnetworks.com weichen.chen@linux.alibaba.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 3 this patch: 3 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 3 this patch: 3 |
| netdev/header_inline | success | Link |
On 17/03/21 15.53, Thadeu Lima de Souza Cascardo wrote: > IFF_POINTOPOINT interfaces use NUD_NOARP entries for IPv6. It's possible to > fill up the neighbour table with enough entries that it will overflow for > valid connections after that. > > This behaviour is more prevalent after commit 58956317c8de ("neighbor: > Improve garbage collection") is applied, as it prevents removal from > entries that are not NUD_FAILED, unless they are more than 5s old. > > Fixes: 58956317c8de (neighbor: Improve garbage collection) > Reported-by: Kasper Dupont <kasperd@gjkwv.06.feb.2021.kasperd.net> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > net/core/neighbour.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > index bbc89c7ffdfd..be5ca411b149 100644 > --- a/net/core/neighbour.c > +++ b/net/core/neighbour.c > @@ -256,6 +256,7 @@ static int neigh_forced_gc(struct neigh_table *tbl) > > write_lock(&n->lock); > if ((n->nud_state == NUD_FAILED) || > + (n->nud_state == NUD_NOARP) || > (tbl->is_multicast && > tbl->is_multicast(n->primary_key)) || > time_after(tref, n->updated)) > -- > 2.27.0 > Is there any update regarding this change? I noticed this regression when it was used in a DoS attack on one of my servers which I had upgraded from Ubuntu 18.04 to 20.04. I have verified that Ubuntu 18.04 is not subject to this attack and Ubuntu 20.04 is vulnerable. I have also verified that the one-line change which Cascardo has provided fixes the vulnerability on Ubuntu 20.04. Kind regards Kasper
On 4/19/21 9:44 AM, Kasper Dupont wrote: > On 17/03/21 15.53, Thadeu Lima de Souza Cascardo wrote: >> IFF_POINTOPOINT interfaces use NUD_NOARP entries for IPv6. It's possible to >> fill up the neighbour table with enough entries that it will overflow for >> valid connections after that. >> >> This behaviour is more prevalent after commit 58956317c8de ("neighbor: >> Improve garbage collection") is applied, as it prevents removal from >> entries that are not NUD_FAILED, unless they are more than 5s old. >> >> Fixes: 58956317c8de (neighbor: Improve garbage collection) >> Reported-by: Kasper Dupont <kasperd@gjkwv.06.feb.2021.kasperd.net> >> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> >> --- >> net/core/neighbour.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/core/neighbour.c b/net/core/neighbour.c >> index bbc89c7ffdfd..be5ca411b149 100644 >> --- a/net/core/neighbour.c >> +++ b/net/core/neighbour.c >> @@ -256,6 +256,7 @@ static int neigh_forced_gc(struct neigh_table *tbl) >> >> write_lock(&n->lock); >> if ((n->nud_state == NUD_FAILED) || >> + (n->nud_state == NUD_NOARP) || >> (tbl->is_multicast && >> tbl->is_multicast(n->primary_key)) || >> time_after(tref, n->updated)) >> -- >> 2.27.0 >> > > Is there any update regarding this change? > > I noticed this regression when it was used in a DoS attack on one of > my servers which I had upgraded from Ubuntu 18.04 to 20.04. > > I have verified that Ubuntu 18.04 is not subject to this attack and > Ubuntu 20.04 is vulnerable. I have also verified that the one-line > change which Cascardo has provided fixes the vulnerability on Ubuntu > 20.04. > your testing included both patches or just this one?
On 19/04/21 10.10, David Ahern wrote: > On 4/19/21 9:44 AM, Kasper Dupont wrote: > > On 17/03/21 15.53, Thadeu Lima de Souza Cascardo wrote: > >> IFF_POINTOPOINT interfaces use NUD_NOARP entries for IPv6. It's possible to > >> fill up the neighbour table with enough entries that it will overflow for > >> valid connections after that. > >> > >> This behaviour is more prevalent after commit 58956317c8de ("neighbor: > >> Improve garbage collection") is applied, as it prevents removal from > >> entries that are not NUD_FAILED, unless they are more than 5s old. > >> > >> Fixes: 58956317c8de (neighbor: Improve garbage collection) > >> Reported-by: Kasper Dupont <kasperd@gjkwv.06.feb.2021.kasperd.net> > >> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > >> --- > >> net/core/neighbour.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/net/core/neighbour.c b/net/core/neighbour.c > >> index bbc89c7ffdfd..be5ca411b149 100644 > >> --- a/net/core/neighbour.c > >> +++ b/net/core/neighbour.c > >> @@ -256,6 +256,7 @@ static int neigh_forced_gc(struct neigh_table *tbl) > >> > >> write_lock(&n->lock); > >> if ((n->nud_state == NUD_FAILED) || > >> + (n->nud_state == NUD_NOARP) || > >> (tbl->is_multicast && > >> tbl->is_multicast(n->primary_key)) || > >> time_after(tref, n->updated)) > >> -- > >> 2.27.0 > >> > > > > Is there any update regarding this change? > > > > I noticed this regression when it was used in a DoS attack on one of > > my servers which I had upgraded from Ubuntu 18.04 to 20.04. > > > > I have verified that Ubuntu 18.04 is not subject to this attack and > > Ubuntu 20.04 is vulnerable. I have also verified that the one-line > > change which Cascardo has provided fixes the vulnerability on Ubuntu > > 20.04. > > > > your testing included both patches or just this one? I applied only this one line change on top of the kernel in Ubuntu 20.04. The behavior I observed was that without the patch the kernel was vulnerable and with that patch I was unable to reproduce the problem. The other longer patch is for a different issue which Cascardo discovered while working on the one I had reported. I don't have an environment set up where I can reproduce the issue addressed by that larger patch.
On 4/19/21 10:52 AM, Kasper Dupont wrote: > On 19/04/21 10.10, David Ahern wrote: >> On 4/19/21 9:44 AM, Kasper Dupont wrote: >>> >>> Is there any update regarding this change? >>> >>> I noticed this regression when it was used in a DoS attack on one of >>> my servers which I had upgraded from Ubuntu 18.04 to 20.04. >>> >>> I have verified that Ubuntu 18.04 is not subject to this attack and >>> Ubuntu 20.04 is vulnerable. I have also verified that the one-line >>> change which Cascardo has provided fixes the vulnerability on Ubuntu >>> 20.04. >>> >> >> your testing included both patches or just this one? > > I applied only this one line change on top of the kernel in Ubuntu > 20.04. The behavior I observed was that without the patch the kernel > was vulnerable and with that patch I was unable to reproduce the > problem. This patch should be re-submitted standalone for -net > > The other longer patch is for a different issue which Cascardo > discovered while working on the one I had reported. I don't have an > environment set up where I can reproduce the issue addressed by that > larger patch. > The first patch is the one I have concerns about.
diff --git a/net/core/neighbour.c b/net/core/neighbour.c index bbc89c7ffdfd..be5ca411b149 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -256,6 +256,7 @@ static int neigh_forced_gc(struct neigh_table *tbl) write_lock(&n->lock); if ((n->nud_state == NUD_FAILED) || + (n->nud_state == NUD_NOARP) || (tbl->is_multicast && tbl->is_multicast(n->primary_key)) || time_after(tref, n->updated))
IFF_POINTOPOINT interfaces use NUD_NOARP entries for IPv6. It's possible to fill up the neighbour table with enough entries that it will overflow for valid connections after that. This behaviour is more prevalent after commit 58956317c8de ("neighbor: Improve garbage collection") is applied, as it prevents removal from entries that are not NUD_FAILED, unless they are more than 5s old. Fixes: 58956317c8de (neighbor: Improve garbage collection) Reported-by: Kasper Dupont <kasperd@gjkwv.06.feb.2021.kasperd.net> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> --- net/core/neighbour.c | 1 + 1 file changed, 1 insertion(+)