I'm not sure who to get review from for this kind of thing.
Kees, you seem to be one of the only people to touch string_helpers.c
at all recently, any ideas?
--b.
On Thu, Jun 20, 2019 at 10:51:07AM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> I'm exposing some information about NFS clients in pseudofiles. I
> expect to eventually have simple tools to help read those pseudofiles.
>
> But it's also helpful if the raw files are human-readable to the extent
> possible. It aids debugging and makes them usable on systems that don't
> have the latest nfs-utils.
>
> A minor challenge there is opaque client-generated protocol objects like
> state owners and client identifiers. Some clients generate those to
> include handy information in plain ascii. But they may also include
> arbitrary byte sequences.
>
> I think the simplest approach is to limit to isprint(c) && isascii(c)
> and escape everything else.
>
> That means you can just cat the file and get something that looks OK.
> Also, I'm trying to keep these files legal YAML, which requires them to
> UTF-8, and this is a simple way to guarantee that.
>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/seq_file.c | 11 +++++++++++
> include/linux/seq_file.h | 1 +
> include/linux/string_helpers.h | 3 +++
> lib/string_helpers.c | 19 +++++++++++++++++++
> 4 files changed, 34 insertions(+)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index abe27ec43176..04f09689cd6d 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -384,6 +384,17 @@ void seq_escape(struct seq_file *m, const char *s, const char *esc)
> }
> EXPORT_SYMBOL(seq_escape);
>
> +void seq_escape_mem_ascii(struct seq_file *m, const char *src, size_t isz)
> +{
> + char *buf;
> + size_t size = seq_get_buf(m, &buf);
> + int ret;
> +
> + ret = string_escape_mem_ascii(src, isz, buf, size);
> + seq_commit(m, ret < size ? ret : -1);
> +}
> +EXPORT_SYMBOL(seq_escape_mem_ascii);
> +
> void seq_vprintf(struct seq_file *m, const char *f, va_list args)
> {
> int len;
> diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
> index a121982af0f5..5998e1f4ff06 100644
> --- a/include/linux/seq_file.h
> +++ b/include/linux/seq_file.h
> @@ -127,6 +127,7 @@ void seq_put_hex_ll(struct seq_file *m, const char *delimiter,
> unsigned long long v, unsigned int width);
>
> void seq_escape(struct seq_file *m, const char *s, const char *esc);
> +void seq_escape_mem_ascii(struct seq_file *m, const char *src, size_t isz);
>
> void seq_hex_dump(struct seq_file *m, const char *prefix_str, int prefix_type,
> int rowsize, int groupsize, const void *buf, size_t len,
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index d23c5030901a..c28955132234 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -54,6 +54,9 @@ static inline int string_unescape_any_inplace(char *buf)
> int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
> unsigned int flags, const char *only);
>
> +int string_escape_mem_ascii(const char *src, size_t isz, char *dst,
> + size_t osz);
> +
> static inline int string_escape_mem_any_np(const char *src, size_t isz,
> char *dst, size_t osz, const char *only)
> {
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 29c490e5d478..9ca19918ca26 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -539,6 +539,25 @@ int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
> }
> EXPORT_SYMBOL(string_escape_mem);
>
> +int string_escape_mem_ascii(const char *src, size_t isz, char *dst,
> + size_t osz)
> +{
> + char *p = dst;
> + char *end = p + osz;
> +
> + while (isz--) {
> + unsigned char c = *src++;
> +
> + if (!isprint(c) || !isascii(c) || c == '"' || c == '\\')
> + escape_hex(c, &p, end);
> + else
> + escape_passthrough(c, &p, end);
> + }
> +
> + return p - dst;
> +}
> +EXPORT_SYMBOL(string_escape_mem_ascii);
> +
> /*
> * Return an allocated string that has been escaped of special characters
> * and double quotes, making it safe to log in quotes.
> --
> 2.21.0
On Fri, Jun 21, 2019 at 01:45:44PM -0400, J. Bruce Fields wrote:
> I'm not sure who to get review from for this kind of thing.
>
> Kees, you seem to be one of the only people to touch string_helpers.c
> at all recently, any ideas?
Hi! Yeah, I'm happy to take a look. Notes below...
>
> --b.
>
> On Thu, Jun 20, 2019 at 10:51:07AM -0400, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <[email protected]>
> >
> > I'm exposing some information about NFS clients in pseudofiles. I
> > expect to eventually have simple tools to help read those pseudofiles.
> >
> > But it's also helpful if the raw files are human-readable to the extent
> > possible. It aids debugging and makes them usable on systems that don't
> > have the latest nfs-utils.
> >
> > A minor challenge there is opaque client-generated protocol objects like
> > state owners and client identifiers. Some clients generate those to
> > include handy information in plain ascii. But they may also include
> > arbitrary byte sequences.
> >
> > I think the simplest approach is to limit to isprint(c) && isascii(c)
> > and escape everything else.
Can you get the same functionality out of sprintf's %pE (escaped
string)? If not, maybe we should expand the flags available?
* - 'E[achnops]' For an escaped buffer, where rules are defined by
* combination
* of the following flags (see string_escape_mem() for
* the
* details):
* a - ESCAPE_ANY
* c - ESCAPE_SPECIAL
* h - ESCAPE_HEX
* n - ESCAPE_NULL
* o - ESCAPE_OCTAL
* p - ESCAPE_NP
* s - ESCAPE_SPACE
* By default ESCAPE_ANY_NP is used.
This doesn't cover escaping >0x7f and " and \
And perhaps I should rework kstrdup_quotable() to have that flag? It's
not currently escaping non-ascii and it probably should. Maybe
"ESCAPE_QUOTABLE" as "q"?
--
Kees Cook