Re: [PATCH v2] seq_file: Unconditionally use vmalloc for buffer

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Mar 15, 2021 at 10:48:51AM -0700, Kees Cook wrote:
> The sysfs interface to seq_file continues to be rather fragile, as seen
> with some recent exploits[1]. Move the seq_file buffer to the vmap area
> (while retaining the accounting flag), since it has guard pages that
> will catch and stop linear overflows. This seems justified given that
> seq_file already uses kvmalloc(), is almost always using a PAGE_SIZE or
> larger allocation, has allocations are normally short lived, and is not
> normally on a performance critical path.

You are attacking the wrong part of it.  Is there any reason for having
seq_get_buf() public in the first place?

For example, the use in blkcg_print_stat() is entirely due to the bogus
->pd_stat_fn() calling conventions.  Fuck scnprintf() games, just pass
seq_file to ->pd_stat_fn() and use seq_printf() instead.  Voila - no
seq_get_buf()/seq_commit()/scnprintf() garbage.

tegra use is no better, AFAICS.  inifinibarf one...  allow me to quote
that gem in full:
static int _driver_stats_seq_show(struct seq_file *s, void *v)
{
        loff_t *spos = v;
        char *buffer;
        u64 *stats = (u64 *)&hfi1_stats;
        size_t sz = seq_get_buf(s, &buffer);

        if (sz < sizeof(u64))
                return SEQ_SKIP;
        /* special case for interrupts */
        if (*spos == 0)
                *(u64 *)buffer = hfi1_sps_ints();
        else
                *(u64 *)buffer = stats[*spos];
        seq_commit(s,  sizeof(u64));
        return 0;
}
Yes, really.  Not to mention that there's seq_write(), what the _hell_
is it using seq_file for in the first place?  Oh, and hfi_stats is
actually this:
struct hfi1_ib_stats {
        __u64 sps_ints; /* number of interrupts handled */
        __u64 sps_errints; /* number of error interrupts */
        __u64 sps_txerrs; /* tx-related packet errors */
        __u64 sps_rcverrs; /* non-crc rcv packet errors */
        __u64 sps_hwerrs; /* hardware errors reported (parity, etc.) */
        __u64 sps_nopiobufs; /* no pio bufs avail from kernel */
        __u64 sps_ctxts; /* number of contexts currently open */
        __u64 sps_lenerrs; /* number of kernel packets where RHF != LRH len */
        __u64 sps_buffull;
        __u64 sps_hdrfull;
};
I won't go into further details - CDA might be dead and buried, but there
should be some limit to public obscenity ;-/

procfs use is borderline - it looks like there might be a good cause
for seq_escape_str().

And sysfs_kf_seq_show()...  Do we want to go through seq_file there at
all?