On Fri, Sep 20, 2013 at 1:08 AM, Jiri Slaby [off-list ref] wrote:

On 09/20/2013 06:09 AM, Tetsuo Handa wrote:

quoted

--- a/fs/proc/consoles.c
+++ b/fs/proc/consoles.c

...

quoted

@@ -47,11 +46,10 @@ static int show_console_dev(struct seq_file *m, void *v)
                      con_flags[a].name : ' ';
      flags[a] = 0;

-     seq_printf(m, "%s%d%n", con->name, con->index, &len);
-     len = 21 - len;
-     if (len < 1)
-             len = 1;
-     seq_printf(m, "%*c%c%c%c (%s)", len, ' ', con->read ? 'R' : '-',
+     seq_setwidth(m, 21 - 1);
+     seq_printf(m, "%s%d", con->name, con->index);
+     seq_pad(m, ' ');
+     seq_printf(m, "%c%c%c (%s)", con->read ? 'R' : '-',
                      con->write ? 'W' : '-', con->unblank ? 'U' : '-',
                      flags);

Hello, do you really need seq_setwidth? It makes it really ugly...

There are a few problems that have been discussed on the various
threads. Namely, we want to minimize the changes to the seq_file
structure and to not add additional work to all the seq_file users
that don't care about padding. If the seq_file calls always track how
far they're written across each call, we add unneeded work to all the
users. To avoid this, we must identify to the seq_file subsystem where
we want to start tracking the length written. To allow this to be
spread across multiple calls (something the %n can't do), we must
record seq->count at some point, and then compare against it at the
point where we want to perform padding.

Or do we need that all? Couldn't we simply have seq_printf_padded? Or
maybe some % modifier in seq_printf to pad the string?

Adding a _padding version of things means we'd have to add it to all
seq_* function that print things (like printing paths, etc). Using
this method, the output doesn't matter. We declare the starting point,
output whatever we need, then perform padding, and continue writing.

I think the declaration/output/pad method seems the least invasive to
existing users of padding, and the highest level of flexibility going
forward for future users.

quoted

--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c

...

quoted

@@ -2548,15 +2549,15 @@ static int fib_route_seq_show(struct seq_file *seq, void *v)
                                       (fi->fib_advmss ?
                                        fi->fib_advmss + 40 : 0),
                                       fi->fib_window,
-                                      fi->fib_rtt >> 3, &len);
+                                      fi->fib_rtt >> 3);
                      else
                              seq_printf(seq,
                                       "*\t%08X\t%08X\t%04X\t%d\t%u\t"
-                                      "%d\t%08X\t%d\t%u\t%u%n",
+                                      "%d\t%08X\t%d\t%u\t%u",
                                       prefix, 0, flags, 0, 0, 0,
-                                      mask, 0, 0, 0, &len);
+                                      mask, 0, 0, 0);

-                     seq_printf(seq, "%*s\n", 127 - len, "");
+                     seq_pad(seq, '\n');

Hmm, seq_pad is unintuitive. I would say it pads the string by '\n'. Of
course it does not, but...

I don't think this is a very serious problem. Currently, the padding
character is always ' ' for all existing callers, so it only makes
sense to make the trailing character an argument.

-Kees

-- 
Kees Cook
Chrome OS Security