Re: [PATCH] net: Adding parameter detection in __ethtool_get_link_ksettings.
> On 8/26/19 9:23 AM, Dongxu Liu wrote:
> The __ethtool_get_link_ksettings symbol will be exported,
> and external users may use an illegal address.
> We should check the parameters before using them,
> otherwise the system will crash.
>
> [ 8980.991134] BUG: unable to handle kernel NULL pointer dereference at
> (null)
> [ 8980.993049] IP: []
> __ethtool_get_link_ksettings+0x27/0x140
> [ 8980.994285] PGD 0
> [ 8980.995013] Oops: [#1] SMP
> [ 8980.995896] Modules linked in: sch_ingress ...
> [ 8981.013220] CPU: 3 PID: 25174 Comm: kworker/3:3 Tainted: G O
> V--- 3.10.0-327.36.58.4.x86_64 #1
> [ 8981.017667] Workqueue: events linkwatch_event
> [ 8981.018652] task: 8800a8348000 ti: 8800b045c000 task.ti:
> 8800b045c000
> [ 8981.020418] RIP: 0010:[] []
> __ethtool_get_link_ksettings+0x27/0x140
> [ 8981.022383] RSP: 0018:8800b045fc88 EFLAGS: 00010202
> [ 8981.023453] RAX: RBX: 8800b045fcac RCX:
>
> [ 8981.024726] RDX: 8800b658f600 RSI: 8800b045fcac RDI:
> 8802296e
> [ 8981.026000] RBP: 8800b045fc98 R08: R09:
> 0001
> [ 8981.027273] R10: 73e0 R11: 082b0cc8adea R12:
> 8802296e
> [ 8981.028561] R13: 8800b566e8c0 R14: 8800b658f600 R15:
> 8800b566e000
> [ 8981.029841] FS: () GS:88023ed8()
> knlGS:
> [ 8981.031715] CS: 0010 DS: ES: CR0: 80050033
> [ 8981.032845] CR2: CR3: b39a9000 CR4:
> 003407e0
> [ 8981.034137] DR0: DR1: DR2:
>
> [ 8981.035427] DR3: DR6: fffe0ff0 DR7:
> 0400
> [ 8981.036702] Stack:
> [ 8981.037406] 8800b658f600 9c40 8800b045fce8
> a047a71d
> [ 8981.039238] 004d 8800b045fcc8 8800b045fd28
> 815cb198
> [ 8981.041070] 8800b045fcd8 810807e6 e8212951
> 0001
> [ 8981.042910] Call Trace:
> [ 8981.043660] [] bond_update_speed_duplex+0x3d/0x90
> [bonding]
> [ 8981.045424] [] ? inetdev_event+0x38/0x530
> [ 8981.046554] [] ? put_online_cpus+0x56/0x80
> [ 8981.047688] [] bond_netdev_event+0x137/0x360 [bonding]
> ...
>
> Signed-off-by: Dongxu Liu
> ---
> net/core/ethtool.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c index
> 6288e69..9a50b64 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -545,6 +545,8 @@ int __ethtool_get_link_ksettings(struct net_device
> *dev, {
> ASSERT_RTNL();
>
> + if (!dev || !dev->ethtool_ops)
> + return -EOPNOTSUPP;
> I do not believe dev can possibly be NULL at this point.
> if (!dev->ethtool_ops->get_link_ksettings)
> return -EOPNOTSUPP;
>
>
> I tried to find an appropriate Fixes: tag.
> It seems this particular bug was added either by
> Fixes: 9856909c2abb ("net: bonding: use __ethtool_get_ksettings")
> or generically in :
> Fixes: 3f1ac7a700d0 ("net: ethtool: add new ETHTOOL_xLINKSETTINGS API")
In fact, "dev->ethtool_ops" is a null pointer in my environment.
I didn't get the case where "dev" is a null pointer.
Maybe "if (!dev->ethtool_ops)" is more accurate for this bug.
I found this bug in version 3.10, the function name was __ethtool_get_settings.
After 3f1ac7a700d0 ("net: ethtool: add new ETHTOOL_xLINKSETTINGS API"),
This function evolved into __ethtool_get_link_ksettings.
[PATCH] net: Adding parameter detection in __ethtool_get_link_ksettings.
The __ethtool_get_link_ksettings symbol will be exported,
and external users may use an illegal address.
We should check the parameters before using them,
otherwise the system will crash.
[ 8980.991134] BUG: unable to handle kernel NULL pointer dereference at
(null)
[ 8980.993049] IP: [] __ethtool_get_link_ksettings+0x27/0x140
[ 8980.994285] PGD 0
[ 8980.995013] Oops: [#1] SMP
[ 8980.995896] Modules linked in: sch_ingress ...
[ 8981.013220] CPU: 3 PID: 25174 Comm: kworker/3:3 Tainted: G O
V--- 3.10.0-327.36.58.4.x86_64 #1
[ 8981.017667] Workqueue: events linkwatch_event
[ 8981.018652] task: 8800a8348000 ti: 8800b045c000 task.ti:
8800b045c000
[ 8981.020418] RIP: 0010:[] []
__ethtool_get_link_ksettings+0x27/0x140
[ 8981.022383] RSP: 0018:8800b045fc88 EFLAGS: 00010202
[ 8981.023453] RAX: RBX: 8800b045fcac RCX:
[ 8981.024726] RDX: 8800b658f600 RSI: 8800b045fcac RDI: 8802296e
[ 8981.026000] RBP: 8800b045fc98 R08: R09: 0001
[ 8981.027273] R10: 73e0 R11: 082b0cc8adea R12: 8802296e
[ 8981.028561] R13: 8800b566e8c0 R14: 8800b658f600 R15: 8800b566e000
[ 8981.029841] FS: () GS:88023ed8()
knlGS:
[ 8981.031715] CS: 0010 DS: ES: CR0: 80050033
[ 8981.032845] CR2: CR3: b39a9000 CR4: 003407e0
[ 8981.034137] DR0: DR1: DR2:
[ 8981.035427] DR3: DR6: fffe0ff0 DR7: 0400
[ 8981.036702] Stack:
[ 8981.037406] 8800b658f600 9c40 8800b045fce8
a047a71d
[ 8981.039238] 004d 8800b045fcc8 8800b045fd28
815cb198
[ 8981.041070] 8800b045fcd8 810807e6 e8212951
0001
[ 8981.042910] Call Trace:
[ 8981.043660] [] bond_update_speed_duplex+0x3d/0x90
[bonding]
[ 8981.045424] [] ? inetdev_event+0x38/0x530
[ 8981.046554] [] ? put_online_cpus+0x56/0x80
[ 8981.047688] [] bond_netdev_event+0x137/0x360 [bonding]
...
Signed-off-by: Dongxu Liu
---
net/core/ethtool.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6288e69..9a50b64 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -545,6 +545,8 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
{
ASSERT_RTNL();
+ if (!dev || !dev->ethtool_ops)
+ return -EOPNOTSUPP;
if (!dev->ethtool_ops->get_link_ksettings)
return -EOPNOTSUPP;
--
2.12.3
[PATCH] net: Add the same IP detection for duplicate address.
The network sends an ARP REQUEST packet to determine
whether there is a host with the same IP.
Windows and some other hosts may send the source IP
address instead of 0.
When IN_DEV_ORCONF(in_dev, DROP_GRATUITOUS_ARP) is enable,
the REQUEST will be dropped.
When IN_DEV_ORCONF(in_dev, DROP_GRATUITOUS_ARP) is disable,
The case should be added to the IP conflict handling process.
Signed-off-by: Dongxu Liu
---
net/ipv4/arp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 05eb42f..a51c921 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -801,7 +801,7 @@ static int arp_process(struct net *net, struct sock *sk,
struct sk_buff *skb)
GFP_ATOMIC);
/* Special case: IPv4 duplicate address detection packet (RFC2131) */
- if (sip == 0) {
+ if (sip == 0 || sip == tip) {
if (arp->ar_op == htons(ARPOP_REQUEST) &&
inet_addr_type_dev_table(net, dev, tip) == RTN_LOCAL &&
!arp_ignore(in_dev, sip, tip))
--
2.12.3
[PATCH] net: Fix detection for IPv4 duplicate address.
The network sends an ARP REQUEST packet to determine
whether there is a host with the same IP.
The source IP address of the packet is 0.
However, Windows may also send the source IP address
to determine, then the source IP address is equal to
the destination IP address.
Signed-off-by: Dongxu Liu
---
net/ipv4/arp.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 05eb42f..944f8e8 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -800,8 +800,11 @@ static int arp_process(struct net *net, struct sock *sk,
struct sk_buff *skb)
iptunnel_metadata_reply(skb_metadata_dst(skb),
GFP_ATOMIC);
- /* Special case: IPv4 duplicate address detection packet (RFC2131) */
- if (sip == 0) {
+/* Special case: IPv4 duplicate address detection packet (RFC2131).
+ * Linux usually sends zero to detect duplication, and windows may
+ * send a same ip (not zero, sip equal to tip) to do this detection.
+ */
+ if (sip == 0 || sip == tip) {
if (arp->ar_op == htons(ARPOP_REQUEST) &&
inet_addr_type_dev_table(net, dev, tip) == RTN_LOCAL &&
!arp_ignore(in_dev, sip, tip))
--
2.12.3
