[PATCH] net: atm: fix update of position index in lec_seq_next

[PATCH] net: atm: fix update of position index in lec_seq_next

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The position index in leq_seq_next is not updated when the next
entry is fetched an no more entries are available. This causes
seq_file to report the following error:

"seq_file: buggy .next function lec_seq_next [lec] did not update
 position index"

Fix this by always updating the position index.

[ Note: this is an ancient 2002 bug, the sha is from the
  tglx/history repo ]

Fixes 4aea2cbff417 ("[ATM]: Move lan seq_file ops to lec.c [1/3]")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/atm/lec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index dbabb65d8b67..7226c784dbe0 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -954,9 +954,8 @@ static void *lec_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
 	struct lec_state *state = seq->private;
 
-	v = lec_get_idx(state, 1);
-	*pos += !!PTR_ERR(v);
-	return v;
+	++*pos;
+	return lec_get_idx(state, 1);
 }
 
 static int lec_seq_show(struct seq_file *seq, void *v)
-- 
2.27.0

Re: [PATCH] net: atm: fix update of position index in lec_seq_next

[Jakub Kicinski]

On Tue, 27 Oct 2020 11:49:25 +0000 Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The position index in leq_seq_next is not updated when the next
> entry is fetched an no more entries are available. This causes
> seq_file to report the following error:
> 
> "seq_file: buggy .next function lec_seq_next [lec] did not update
>  position index"
> 
> Fix this by always updating the position index.
> 
> [ Note: this is an ancient 2002 bug, the sha is from the
>   tglx/history repo ]
> 
> Fixes 4aea2cbff417 ("[ATM]: Move lan seq_file ops to lec.c [1/3]")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, very sneaky there with the lack of a colon on the Fixes tag :)


BTW looking at seq_read() it seems to eat the potential error code from
->next, doesn't it?

@@ -254,9 +254,11 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
        }
        m->op->stop(m, p);
        n = min(m->count, size);
-       err = copy_to_user(buf, m->buf, n);
-       if (err)
-               goto Efault;
+       if (n) {
+               err = copy_to_user(buf, m->buf, n);
+               if (err)
+                       goto Efault;
+       }
        copied += n;
        m->count -= n;
        m->from = n;

Maybe? Or at least:

@@ -239,10 +239,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
                                            m->op->next);
                        m->index++;
                }
-               if (!p || IS_ERR(p)) {
-                       err = PTR_ERR(p);
+               if (!p || IS_ERR(p))
                        break;
-               }
                if (m->count >= size)
                        break;
                err = m->op->show(m, p);