* [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
@ 2014-02-17 11:31 Wang Weidong
2014-02-17 17:09 ` Daniel Borkmann
2014-02-18 1:42 ` [PATCH v2] " Wang Weidong
0 siblings, 2 replies; 7+ messages in thread
From: Wang Weidong @ 2014-02-17 11:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev
some drivers maybe not implement the ethtool_ops with only
set NULL. So when call the ethtool cmds will lead to a
'NULL pointer dereference'.
So add a checking in dev_ethtool.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/core/ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 30071de..f418dcb 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
return -EPERM;
}
+
+ if (!dev->ethtool_ops)
+ return -EOPNOTSUPP;
if (dev->ethtool_ops->begin) {
rc = dev->ethtool_ops->begin(dev);
--
1.7.12
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
2014-02-17 11:31 [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool Wang Weidong
@ 2014-02-17 17:09 ` Daniel Borkmann
2014-02-18 1:20 ` Wang Weidong
2014-02-18 1:42 ` [PATCH v2] " Wang Weidong
1 sibling, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2014-02-17 17:09 UTC (permalink / raw)
To: Wang Weidong; +Cc: David Miller, netdev
On 02/17/2014 12:31 PM, Wang Weidong wrote:
> some drivers maybe not implement the ethtool_ops with only
> set NULL. So when call the ethtool cmds will lead to a
> 'NULL pointer dereference'.
>
> So add a checking in dev_ethtool.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> net/core/ethtool.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 30071de..f418dcb 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> return -EPERM;
> }
> +
You have a trailing whitespace/tab in the line above. Please
use checkpatch for detecting such things.
Can you be more specific with "some drivers"? Any driver that
is in the mainline tree?
> + if (!dev->ethtool_ops)
> + return -EOPNOTSUPP;
>
> if (dev->ethtool_ops->begin) {
> rc = dev->ethtool_ops->begin(dev);
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
2014-02-17 17:09 ` Daniel Borkmann
@ 2014-02-18 1:20 ` Wang Weidong
2014-02-18 9:22 ` Daniel Borkmann
0 siblings, 1 reply; 7+ messages in thread
From: Wang Weidong @ 2014-02-18 1:20 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev
On 2014/2/18 1:09, Daniel Borkmann wrote:
> On 02/17/2014 12:31 PM, Wang Weidong wrote:
>> some drivers maybe not implement the ethtool_ops with only
>> set NULL. So when call the ethtool cmds will lead to a
>> 'NULL pointer dereference'.
>>
>> So add a checking in dev_ethtool.
>>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>> net/core/ethtool.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 30071de..f418dcb 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>> if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
>> return -EPERM;
>> }
>> +
>
> You have a trailing whitespace/tab in the line above. Please
> use checkpatch for detecting such things.
>
Sorry for that. I will fix it soon.
> Can you be more specific with "some drivers"? Any driver that
> is in the mainline tree?
>
No. My team implements a driver without considering the ethtool_ops.
So I got the problem.
>> + if (!dev->ethtool_ops)
>> + return -EOPNOTSUPP;
>>
>> if (dev->ethtool_ops->begin) {
>> rc = dev->ethtool_ops->begin(dev);
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
2014-02-18 1:20 ` Wang Weidong
@ 2014-02-18 9:22 ` Daniel Borkmann
2014-02-18 10:40 ` Wang Weidong
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2014-02-18 9:22 UTC (permalink / raw)
To: Wang Weidong; +Cc: David Miller, netdev
On 02/18/2014 02:20 AM, Wang Weidong wrote:
> On 2014/2/18 1:09, Daniel Borkmann wrote:
>> On 02/17/2014 12:31 PM, Wang Weidong wrote:
>>> some drivers maybe not implement the ethtool_ops with only
>>> set NULL. So when call the ethtool cmds will lead to a
>>> 'NULL pointer dereference'.
>>>
>>> So add a checking in dev_ethtool.
>>>
>>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>>> ---
>>> net/core/ethtool.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>>> index 30071de..f418dcb 100644
>>> --- a/net/core/ethtool.c
>>> +++ b/net/core/ethtool.c
>>> @@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>>> if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
>>> return -EPERM;
>>> }
>>> +
>>
>> You have a trailing whitespace/tab in the line above. Please
>> use checkpatch for detecting such things.
>>
> Sorry for that. I will fix it soon.
>
>> Can you be more specific with "some drivers"? Any driver that
>> is in the mainline tree?
>>
> No. My team implements a driver without considering the ethtool_ops.
> So I got the problem.
If it's code that is out of the mainline tree, then why should the
kernel support that? Submit your driver to the tree first, and then
such a change could be considered. Or, even better, let them implement
ethtool ops/stubs.
>>> + if (!dev->ethtool_ops)
>>> + return -EOPNOTSUPP;
>>>
>>> if (dev->ethtool_ops->begin) {
>>> rc = dev->ethtool_ops->begin(dev);
>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
2014-02-18 9:22 ` Daniel Borkmann
@ 2014-02-18 10:40 ` Wang Weidong
0 siblings, 0 replies; 7+ messages in thread
From: Wang Weidong @ 2014-02-18 10:40 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev
On 2014/2/18 17:22, Daniel Borkmann wrote:
> On 02/18/2014 02:20 AM, Wang Weidong wrote:
>> On 2014/2/18 1:09, Daniel Borkmann wrote:
>>> On 02/17/2014 12:31 PM, Wang Weidong wrote:
>>>> some drivers maybe not implement the ethtool_ops with only
>>>> set NULL. So when call the ethtool cmds will lead to a
>>>> 'NULL pointer dereference'.
>>>>
>>>> So add a checking in dev_ethtool.
>>>>
>>>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>>>> ---
>>>> net/core/ethtool.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>>>> index 30071de..f418dcb 100644
>>>> --- a/net/core/ethtool.c
>>>> +++ b/net/core/ethtool.c
>>>> @@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>>>> if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
>>>> return -EPERM;
>>>> }
>>>> +
>>>
>>> You have a trailing whitespace/tab in the line above. Please
>>> use checkpatch for detecting such things.
>>>
>> Sorry for that. I will fix it soon.
>>
>>> Can you be more specific with "some drivers"? Any driver that
>>> is in the mainline tree?
>>>
>> No. My team implements a driver without considering the ethtool_ops.
>> So I got the problem.
>
> If it's code that is out of the mainline tree, then why should the
> kernel support that? Submit your driver to the tree first, and then
> such a change could be considered. Or, even better, let them implement
> ethtool ops/stubs.
>
Thanks for your reply. I will suggest them to implement ethtool ops.
Regards
Wang
>>>> + if (!dev->ethtool_ops)
>>>> + return -EOPNOTSUPP;
>>>>
>>>> if (dev->ethtool_ops->begin) {
>>>> rc = dev->ethtool_ops->begin(dev);
>>>>
>>>
>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] ethtool: check the ethtool_ops is NULL in dev_ethtool
2014-02-17 11:31 [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool Wang Weidong
2014-02-17 17:09 ` Daniel Borkmann
@ 2014-02-18 1:42 ` Wang Weidong
2014-02-18 10:41 ` Wang Weidong
1 sibling, 1 reply; 7+ messages in thread
From: Wang Weidong @ 2014-02-18 1:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Daniel Borkmann
some drivers maybe not implement the ethtool_ops with only
set NULL to ethtool_ops. So when call the ethtool devx will
lead to a 'NULL pointer dereference'.
So add a check in dev_ethtool
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
Change note:
v2: fix a trailing whitespace/tab pointed out by Daniel
---
net/core/ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 30071de..c8cfd8f 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1500,6 +1500,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
return -EPERM;
}
+ if (!dev->ethtool_ops)
+ return -EOPNOTSUPP;
+
if (dev->ethtool_ops->begin) {
rc = dev->ethtool_ops->begin(dev);
if (rc < 0)
--
1.7.12
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] ethtool: check the ethtool_ops is NULL in dev_ethtool
2014-02-18 1:42 ` [PATCH v2] " Wang Weidong
@ 2014-02-18 10:41 ` Wang Weidong
0 siblings, 0 replies; 7+ messages in thread
From: Wang Weidong @ 2014-02-18 10:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Daniel Borkmann
Just ignore it.
Regards
Wang
On 2014/2/18 9:42, Wang Weidong wrote:
> some drivers maybe not implement the ethtool_ops with only
> set NULL to ethtool_ops. So when call the ethtool devx will
> lead to a 'NULL pointer dereference'.
>
> So add a check in dev_ethtool
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> Change note:
>
> v2: fix a trailing whitespace/tab pointed out by Daniel
>
> ---
> net/core/ethtool.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 30071de..c8cfd8f 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1500,6 +1500,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> return -EPERM;
> }
>
> + if (!dev->ethtool_ops)
> + return -EOPNOTSUPP;
> +
> if (dev->ethtool_ops->begin) {
> rc = dev->ethtool_ops->begin(dev);
> if (rc < 0)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-18 10:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-17 11:31 [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool Wang Weidong
2014-02-17 17:09 ` Daniel Borkmann
2014-02-18 1:20 ` Wang Weidong
2014-02-18 9:22 ` Daniel Borkmann
2014-02-18 10:40 ` Wang Weidong
2014-02-18 1:42 ` [PATCH v2] " Wang Weidong
2014-02-18 10:41 ` Wang Weidong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.