Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 5 Sep 2019 15:34:03 -0700
From:   Kees Cook <keescook@chromium.org>
To:     "J. Bruce Fields" <bfields@redhat.com>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 9/9] Remove string_escape_mem_ascii
Message-ID: <201909051532.3062C94A1@keescook>
References: <20190905193604.GC31247@fieldses.org>
 <1567712673-1629-1-git-send-email-bfields@redhat.com>
 <1567712673-1629-9-git-send-email-bfields@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1567712673-1629-9-git-send-email-bfields@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Sep 05, 2019 at 03:44:33PM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> It's easier to do this in string_escape_mem now.
> 
> Might also consider non-ascii and quote-mark sprintf modifiers and then
> we might make do with seq_printk.

With '\' always handled, it can be dropped from the "esc" args below. I
wonder if ESCAPE_QUOTES is needed or if "esc" can just continue to be
used for that.

(Also, do we want to add the "hex escape" modifier back to snprintf's
%pE stuff?)

-Kees

> ---
>  fs/seq_file.c                  |  3 ++-
>  include/linux/string_helpers.h |  3 +--
>  lib/string_helpers.c           | 24 ++++--------------------
>  3 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 63e5a7c4dbf7..0e45a25523ad 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -390,7 +390,8 @@ void seq_escape_mem_ascii(struct seq_file *m, const char *src, size_t isz)
>  	size_t size = seq_get_buf(m, &buf);
>  	int ret;
>  
> -	ret = string_escape_mem_ascii(src, isz, buf, size);
> +	ret = string_escape_mem(src, isz, buf, size, ESCAPE_NP|ESCAPE_NONASCII|
> +				ESCAPE_STYLE_SLASH|ESCAPE_STYLE_HEX, "\"\\");
>  	seq_commit(m, ret < size ? ret : -1);
>  }
>  EXPORT_SYMBOL(seq_escape_mem_ascii);
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index 5d350f7f6874..f3388591d83f 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -43,6 +43,7 @@ static inline int string_unescape_any_inplace(char *buf)
>  
>  #define ESCAPE_SPECIAL		0x01
>  #define ESCAPE_NP		0x02
> +#define ESCAPE_NONASCII		0x04
>  #define ESCAPE_ANY_NP		(ESCAPE_SPECIAL | ESCAPE_NP)
>  #define ESCAPE_STYLE_SLASH	0x20
>  #define ESCAPE_STYLE_OCTAL	0x40
> @@ -52,8 +53,6 @@ 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_str(const char *src, char *dst, size_t sz,
>  		unsigned int flags, const char *only)
>  {
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 6f553f893fda..1dacf76eada0 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -439,6 +439,8 @@ static bool is_special(char c)
>   *		'\a' - alert (BEL)
>   *		'\e' - escape
>   *		'\0' - null
> + *	%ESCAPE_NONASCII:
> + *		escape characters with the high bit set
>   *	%ESCAPE_NP:
>   *		escape only non-printable characters (checked by isprint)
>   *	%ESCAPE_ANY_NP:
> @@ -468,7 +470,8 @@ int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
>  
>  		if ((is_dict && strchr(esc, c)) ||
>  		    (flags & ESCAPE_NP && !isprint(c)) ||
> -		    (flags & ESCAPE_SPECIAL && is_special(c))) {
> +		    (flags & ESCAPE_SPECIAL && is_special(c)) ||
> +		    (flags & ESCAPE_NONASCII && !isascii(c))) {
>  
>  			if (flags & ESCAPE_STYLE_SLASH &&
>  					escape_special(c, &p, end))
> @@ -491,25 +494,6 @@ 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
> 

-- 
Kees Cook