* [PATCH] net/core: Fix seeking in /proc/net/dev
@ 2012-04-06 17:05 John Keeping
2012-04-06 17:14 ` Mihai Maruseac
2012-04-06 17:18 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: John Keeping @ 2012-04-06 17:05 UTC (permalink / raw)
To: netdev; +Cc: John Keeping, linux-kernel, Mihai Maruseac
Commit f04565ddf52e4 (dev: use name hash for dev_seq_ops) introduced
code that fails to check the requested position when getting an item for
/proc/net/dev. This means that any code which seeks within this file is
likely to receive corrupted data.
A test case for this is to use the read builtin in bash:
$ while read line; do echo "$line"; done </proc/net/dev | cut -c-20
Inter-| Receive
face |bytes packe
virbr0: 20706
0
lo: 2329335 10305
eth0: 0
compared to just cat'ing the file:
$ cat /proc/net/dev | cut -c-20
Inter-| Receive
face |bytes pack
lo: 2329335 10
virbr0: 20706
sit0: 0
wlan0: 1727234745 1
eth0: 0
This patch takes the sledgehammer approach of starting again from the
beginning if asked to seek backwards.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
I have made the minimal change required to fix the bug here. If desired I
can spend some more time and enhance the dev_from_new_bucket and
dev_from_same_bucket functions to support walking backwards as well as
forwards through the items in the table.
---
net/core/dev.c | 24 +++++++++++++++++++-----
1 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6ca32f6..66b1e891 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4040,6 +4040,7 @@ static int dev_ifconf(struct net *net, char __user *arg)
struct dev_iter_state {
struct seq_net_private p;
+ loff_t expected_pos; /* current index */
unsigned int pos; /* bucket << BUCKET_SPACE + offset */
};
@@ -4096,24 +4097,37 @@ static inline struct net_device *dev_from_new_bucket(struct seq_file *seq)
void *dev_seq_start(struct seq_file *seq, loff_t *pos)
__acquires(RCU)
{
+ struct net_device *dev;
struct dev_iter_state *state = seq->private;
rcu_read_lock();
- if (!*pos)
+ if (!*pos) {
+ state->expected_pos = 0;
+ state->pos = 0;
return SEQ_START_TOKEN;
+ }
- /* check for end of the hash */
- if (state->pos == 0 && *pos > 1)
- return NULL;
+ /* If we're asked for something behind where we are, start again. */
+ if (state->expected_pos >= *pos) {
+ state->expected_pos = 0;
+ state->pos = 0;
+ }
- return dev_from_new_bucket(seq);
+ do {
+ dev = dev_from_new_bucket(seq);
+ ++state->expected_pos;
+ } while (dev && state->expected_pos < *pos);
+
+ return dev;
}
void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
struct net_device *dev;
+ struct dev_iter_state *state = seq->private;
++*pos;
+ state->expected_pos = *pos;
if (v == SEQ_START_TOKEN)
return dev_from_new_bucket(seq);
--
1.7.8.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net/core: Fix seeking in /proc/net/dev
2012-04-06 17:05 [PATCH] net/core: Fix seeking in /proc/net/dev John Keeping
@ 2012-04-06 17:14 ` Mihai Maruseac
2012-04-06 17:41 ` Eric Dumazet
2012-04-06 17:18 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Mihai Maruseac @ 2012-04-06 17:14 UTC (permalink / raw)
To: John Keeping; +Cc: netdev, linux-kernel, Mihai Maruseac, Daniel Baluta
On Fri, Apr 6, 2012 at 8:05 PM, John Keeping <john@keeping.me.uk> wrote:
> Commit f04565ddf52e4 (dev: use name hash for dev_seq_ops) introduced
> code that fails to check the requested position when getting an item for
> /proc/net/dev. This means that any code which seeks within this file is
> likely to receive corrupted data.
>
> A test case for this is to use the read builtin in bash:
>
> $ while read line; do echo "$line"; done </proc/net/dev | cut -c-20
> Inter-| Receive
> face |bytes packe
> virbr0: 20706
> 0
> lo: 2329335 10305
> eth0: 0
>
> compared to just cat'ing the file:
>
> $ cat /proc/net/dev | cut -c-20
> Inter-| Receive
> face |bytes pack
> lo: 2329335 10
> virbr0: 20706
> sit0: 0
> wlan0: 1727234745 1
> eth0: 0
>
> This patch takes the sledgehammer approach of starting again from the
> beginning if asked to seek backwards.
>
> Signed-off-by: John Keeping <john@keeping.me.uk>
>
> ---
> I have made the minimal change required to fix the bug here. If desired I
> can spend some more time and enhance the dev_from_new_bucket and
> dev_from_same_bucket functions to support walking backwards as well as
> forwards through the items in the table.
>
> ---
> net/core/dev.c | 24 +++++++++++++++++++-----
> 1 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6ca32f6..66b1e891 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4040,6 +4040,7 @@ static int dev_ifconf(struct net *net, char __user *arg)
>
> struct dev_iter_state {
> struct seq_net_private p;
> + loff_t expected_pos; /* current index */
> unsigned int pos; /* bucket << BUCKET_SPACE + offset */
> };
>
> @@ -4096,24 +4097,37 @@ static inline struct net_device *dev_from_new_bucket(struct seq_file *seq)
> void *dev_seq_start(struct seq_file *seq, loff_t *pos)
> __acquires(RCU)
> {
> + struct net_device *dev;
> struct dev_iter_state *state = seq->private;
>
> rcu_read_lock();
> - if (!*pos)
> + if (!*pos) {
> + state->expected_pos = 0;
> + state->pos = 0;
> return SEQ_START_TOKEN;
> + }
>
> - /* check for end of the hash */
> - if (state->pos == 0 && *pos > 1)
> - return NULL;
> + /* If we're asked for something behind where we are, start again. */
> + if (state->expected_pos >= *pos) {
> + state->expected_pos = 0;
> + state->pos = 0;
> + }
>
> - return dev_from_new_bucket(seq);
> + do {
> + dev = dev_from_new_bucket(seq);
> + ++state->expected_pos;
> + } while (dev && state->expected_pos < *pos);
> +
> + return dev;
> }
>
> void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> {
> struct net_device *dev;
> + struct dev_iter_state *state = seq->private;
>
> ++*pos;
> + state->expected_pos = *pos;
>
> if (v == SEQ_START_TOKEN)
> return dev_from_new_bucket(seq);
> --
> 1.7.8.5
>
Looks good to me. However, Eric just submitted a patch here with other
changes caused by a logic error in the original patch.
Now I understand why those resets to the beginning were there (though
they are very rare)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/core: Fix seeking in /proc/net/dev
2012-04-06 17:14 ` Mihai Maruseac
@ 2012-04-06 17:41 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2012-04-06 17:41 UTC (permalink / raw)
To: Mihai Maruseac
Cc: John Keeping, netdev, linux-kernel, Mihai Maruseac, Daniel Baluta
On Fri, 2012-04-06 at 20:14 +0300, Mihai Maruseac wrote:
> On Fri, Apr 6, 2012 at 8:05 PM, John Keeping <john@keeping.me.uk> wrote:
> > Commit f04565ddf52e4 (dev: use name hash for dev_seq_ops) introduced
> > code that fails to check the requested position when getting an item for
> > /proc/net/dev. This means that any code which seeks within this file is
> > likely to receive corrupted data.
> >
> > A test case for this is to use the read builtin in bash:
> >
...
> Looks good to me. However, Eric just submitted a patch here with other
> changes caused by a logic error in the original patch.
Hmm, I think my patch fixed this lseek issue as well.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/core: Fix seeking in /proc/net/dev
2012-04-06 17:05 [PATCH] net/core: Fix seeking in /proc/net/dev John Keeping
2012-04-06 17:14 ` Mihai Maruseac
@ 2012-04-06 17:18 ` David Miller
2012-04-06 17:43 ` Eric Dumazet
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2012-04-06 17:18 UTC (permalink / raw)
To: john; +Cc: netdev, linux-kernel, mihai.maruseac
Eric Dumazet already fixed this the other day.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-06 18:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-06 17:05 [PATCH] net/core: Fix seeking in /proc/net/dev John Keeping
2012-04-06 17:14 ` Mihai Maruseac
2012-04-06 17:41 ` Eric Dumazet
2012-04-06 17:18 ` David Miller
2012-04-06 17:43 ` Eric Dumazet
2012-04-06 18:19 ` John Keeping
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).