2016-04-07 07:22:56

by Vegard Nossum

[permalink] [raw]
Subject: [PATCH] net: mark DECnet as broken

There are NULL pointer dereference bugs in DECnet which can be triggered
by unprivileged users and have been reported multiple times to LKML,
however nobody seems confident enough in the proposed fixes to merge them
and the consensus seems to be that nobody cares enough about DECnet to
see it fixed anyway.

To shield unsuspecting users from the possible DOS, we should mark this
BROKEN until somebody who actually uses this code can fix it.

Signed-off-by: Vegard Nossum <[email protected]>
Link: https://lkml.org/lkml/2015/12/17/666
Cc: Eric Dumazet <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: David Miller <[email protected]>
---
net/decnet/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/decnet/Kconfig b/net/decnet/Kconfig
index f3393e1..b040172 100644
--- a/net/decnet/Kconfig
+++ b/net/decnet/Kconfig
@@ -3,6 +3,7 @@
#
config DECNET
tristate "DECnet Support"
+ depends on BROKEN
---help---
The DECnet networking protocol was used in many products made by
Digital (now Compaq). It provides reliable stream and sequenced
--
1.9.1


2016-04-07 07:50:25

by James Cameron

[permalink] [raw]
Subject: Re: [PATCH] net: mark DECnet as broken

On Thu, Apr 07, 2016 at 09:22:43AM +0200, Vegard Nossum wrote:
> There are NULL pointer dereference bugs in DECnet which can be triggered
> by unprivileged users and have been reported multiple times to LKML,
> however nobody seems confident enough in the proposed fixes to merge them
> and the consensus seems to be that nobody cares enough about DECnet to
> see it fixed anyway.
>
> To shield unsuspecting users from the possible DOS, we should mark this
> BROKEN until somebody who actually uses this code can fix it.
>
> Signed-off-by: Vegard Nossum <[email protected]>
> Link: https://lkml.org/lkml/2015/12/17/666
> Cc: Eric Dumazet <[email protected]>
> Cc: Sasha Levin <[email protected]>
> Cc: David Miller <[email protected]>

Reviewed-by: James Cameron <[email protected]>

(An old DECnet application programmer from way back, ah what fun!)

> ---
> net/decnet/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/decnet/Kconfig b/net/decnet/Kconfig
> index f3393e1..b040172 100644
> --- a/net/decnet/Kconfig
> +++ b/net/decnet/Kconfig
> @@ -3,6 +3,7 @@
> #
> config DECNET
> tristate "DECnet Support"
> + depends on BROKEN
> ---help---
> The DECnet networking protocol was used in many products made by
> Digital (now Compaq). It provides reliable stream and sequenced

fwiw, then Compaq merged into HP.

> --
> 1.9.1
>

--
James Cameron
http://quozl.netrek.org/

2016-04-07 14:01:38

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH] net: mark DECnet as broken

On Thu, 7 Apr 2016 09:22:43 +0200
Vegard Nossum <[email protected]> wrote:

> There are NULL pointer dereference bugs in DECnet which can be triggered
> by unprivileged users and have been reported multiple times to LKML,
> however nobody seems confident enough in the proposed fixes to merge them
> and the consensus seems to be that nobody cares enough about DECnet to
> see it fixed anyway.
>
> To shield unsuspecting users from the possible DOS, we should mark this
> BROKEN until somebody who actually uses this code can fix it.

How about consigning it to staging at this point ?

Alan

2016-04-07 16:21:28

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net: mark DECnet as broken

From: One Thousand Gnomes <[email protected]>
Date: Thu, 7 Apr 2016 15:01:20 +0100

> On Thu, 7 Apr 2016 09:22:43 +0200
> Vegard Nossum <[email protected]> wrote:
>
>> There are NULL pointer dereference bugs in DECnet which can be triggered
>> by unprivileged users and have been reported multiple times to LKML,
>> however nobody seems confident enough in the proposed fixes to merge them
>> and the consensus seems to be that nobody cares enough about DECnet to
>> see it fixed anyway.
>>
>> To shield unsuspecting users from the possible DOS, we should mark this
>> BROKEN until somebody who actually uses this code can fix it.
>
> How about consigning it to staging at this point ?

Staging is a one way facility in my opinion.

I saw we just fix the NULL dereference.

2016-04-11 03:02:11

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net: mark DECnet as broken

From: Vegard Nossum <[email protected]>
Date: Thu, 7 Apr 2016 09:22:43 +0200

> There are NULL pointer dereference bugs in DECnet which can be triggered
> by unprivileged users and have been reported multiple times to LKML,
> however nobody seems confident enough in the proposed fixes to merge them
> and the consensus seems to be that nobody cares enough about DECnet to
> see it fixed anyway.
>
> To shield unsuspecting users from the possible DOS, we should mark this
> BROKEN until somebody who actually uses this code can fix it.
>
> Signed-off-by: Vegard Nossum <[email protected]>
> Link: https://lkml.org/lkml/2015/12/17/666

As stated, I'm not applying this, and rather I am fixing this as
below:

====================
[PATCH] decnet: Do not build routes to devices without decnet private data.

In particular, make sure we check for decnet private presence
for loopback devices.

Signed-off-by: David S. Miller <[email protected]>
---
net/decnet/dn_route.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 607a14f..b1dc096 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1034,10 +1034,13 @@ source_ok:
if (!fld.daddr) {
fld.daddr = fld.saddr;

- err = -EADDRNOTAVAIL;
if (dev_out)
dev_put(dev_out);
+ err = -EINVAL;
dev_out = init_net.loopback_dev;
+ if (!dev_out->dn_ptr)
+ goto out;
+ err = -EADDRNOTAVAIL;
dev_hold(dev_out);
if (!fld.daddr) {
fld.daddr =
@@ -1110,6 +1113,8 @@ source_ok:
if (dev_out == NULL)
goto out;
dn_db = rcu_dereference_raw(dev_out->dn_ptr);
+ if (!dn_db)
+ goto e_inval;
/* Possible improvement - check all devices for local addr */
if (dn_dev_islocal(dev_out, fld.daddr)) {
dev_put(dev_out);
@@ -1151,6 +1156,8 @@ select_source:
dev_put(dev_out);
dev_out = init_net.loopback_dev;
dev_hold(dev_out);
+ if (!dev_out->dn_ptr)
+ goto e_inval;
fld.flowidn_oif = dev_out->ifindex;
if (res.fi)
dn_fib_info_put(res.fi);
--
2.1.0