Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1696025 > unrolled thread
| Started by | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| First post | 2017-07-25 20:40 +0200 |
| Last post | 2017-07-27 02:10 +0200 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] netpoll: Fix device name check in netpoll_setup() Matthias Kaehlcke <mka@chromium.org> - 2017-07-25 20:40 +0200
Re: [PATCH] netpoll: Fix device name check in netpoll_setup() Doug Anderson <dianders@chromium.org> - 2017-07-26 21:00 +0200
Re: [PATCH] netpoll: Fix device name check in netpoll_setup() Cong Wang <xiyou.wangcong@gmail.com> - 2017-07-26 23:00 +0200
Re: [PATCH] netpoll: Fix device name check in netpoll_setup() David Miller <davem@davemloft.net> - 2017-07-27 02:10 +0200
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-07-25 20:40 +0200 |
| Subject | [PATCH] netpoll: Fix device name check in netpoll_setup() |
| Message-ID | <u7cUp-6m2-15@gated-at.bofh.it> |
Apparently netpoll_setup() assumes that netpoll.dev_name is a pointer
when checking if the device name is set:
if (np->dev_name) {
...
However the field is a character array, therefore the condition always
yields true. Check instead whether the first byte of the array has a
non-zero value.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
net/core/netpoll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 8357f164c660..912731bed7b7 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -666,7 +666,7 @@ int netpoll_setup(struct netpoll *np)
int err;
rtnl_lock();
- if (np->dev_name) {
+ if (np->dev_name[0]) {
struct net *net = current->nsproxy->net_ns;
ndev = __dev_get_by_name(net, np->dev_name);
}
--
2.14.0.rc0.284.gd933b75aa4-goog
[toc] | [next] | [standalone]
| From | Doug Anderson <dianders@chromium.org> |
|---|---|
| Date | 2017-07-26 21:00 +0200 |
| Message-ID | <u7zHk-3ZT-23@gated-at.bofh.it> |
| In reply to | #1696025 |
Hi,
On Tue, Jul 25, 2017 at 11:36 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Apparently netpoll_setup() assumes that netpoll.dev_name is a pointer
> when checking if the device name is set:
>
> if (np->dev_name) {
> ...
>
> However the field is a character array, therefore the condition always
> yields true. Check instead whether the first byte of the array has a
> non-zero value.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> net/core/netpoll.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 8357f164c660..912731bed7b7 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -666,7 +666,7 @@ int netpoll_setup(struct netpoll *np)
> int err;
>
> rtnl_lock();
> - if (np->dev_name) {
> + if (np->dev_name[0]) {
> struct net *net = current->nsproxy->net_ns;
> ndev = __dev_get_by_name(net, np->dev_name);
> }
It's really up to the maintainer of the code, but my first instinct
here would be to instead remove the "if" test unless we really expect
dev->dev_name to be blank in lots of cases. It will slightly slow
down the error case but should avoid an "if" test in the non-error
case. By definition it should be safe since currently the "if" test
should always evaluate to true.
-Doug
[toc] | [prev] | [next] | [standalone]
| From | Cong Wang <xiyou.wangcong@gmail.com> |
|---|---|
| Date | 2017-07-26 23:00 +0200 |
| Message-ID | <u7Bzs-5cJ-9@gated-at.bofh.it> |
| In reply to | #1697487 |
On Wed, Jul 26, 2017 at 11:44 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Tue, Jul 25, 2017 at 11:36 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
>> Apparently netpoll_setup() assumes that netpoll.dev_name is a pointer
>> when checking if the device name is set:
>>
>> if (np->dev_name) {
>> ...
>>
>> However the field is a character array, therefore the condition always
>> yields true. Check instead whether the first byte of the array has a
>> non-zero value.
>>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> net/core/netpoll.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
>> index 8357f164c660..912731bed7b7 100644
>> --- a/net/core/netpoll.c
>> +++ b/net/core/netpoll.c
>> @@ -666,7 +666,7 @@ int netpoll_setup(struct netpoll *np)
>> int err;
>>
>> rtnl_lock();
>> - if (np->dev_name) {
>> + if (np->dev_name[0]) {
>> struct net *net = current->nsproxy->net_ns;
>> ndev = __dev_get_by_name(net, np->dev_name);
>> }
>
> It's really up to the maintainer of the code, but my first instinct
> here would be to instead remove the "if" test unless we really expect
> dev->dev_name to be blank in lots of cases. It will slightly slow
> down the error case but should avoid an "if" test in the non-error
> case. By definition it should be safe since currently the "if" test
> should always evaluate to true.
>
netconsole could set this dev_name to empty via configfs,
so this patch is correct.
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2017-07-27 02:10 +0200 |
| Message-ID | <u7Exj-7hl-3@gated-at.bofh.it> |
| In reply to | #1696025 |
From: Matthias Kaehlcke <mka@chromium.org>
Date: Tue, 25 Jul 2017 11:36:25 -0700
> Apparently netpoll_setup() assumes that netpoll.dev_name is a pointer
> when checking if the device name is set:
>
> if (np->dev_name) {
> ...
>
> However the field is a character array, therefore the condition always
> yields true. Check instead whether the first byte of the array has a
> non-zero value.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Applied, thanks a lot.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web