Re: [PATCH 2/2] isofs: bound the Joliet iocharset conversion by the output buffer

[Jan Kara]

On Tue 22-09-26 23:55:24, Matthias Goergens wrote:
> uni16_to_x8() has no output-size parameter.  It passes
> NLS_MAX_CHARSET_SIZE to uni2char() on every iteration, which bounds what
> that one character may write but says nothing about how much of the
> buffer is left, and then writes a terminator wherever it stopped.
> 
> Nothing bounds the total.  It fits today only because the caller's input
> is bounded: de->name_len is a single byte, so at most 255 >> 1 units,
> each expanding to at most NLS_MAX_CHARSET_SIZE bytes.  The previous
> patch asserts that relationship at build time, but the conversion itself
> should not depend on it.
> 
> Pass the buffer size in and hand uni2char() the space that actually
> remains, stopping on -ENAMETOOLONG, as fs/hfsplus/unicode.c does.
> 
> The result is unchanged for every name a valid image can carry; only a
> caller that passed a smaller buffer would now truncate rather than
> overrun.  Listings of plain, Rock Ridge, Joliet, Rock-Ridge-plus-Joliet
> and zisofs images are unchanged.
> 
> Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>

This looks mostly good except that I'd prefer get_joliet_filename() gets
the buffer length as an argument and just passes it down instead of
hardcoding it. That way the place allocating the buffer can just pass
proper size.

								Honza

> ---
>  fs/isofs/joliet.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/isofs/joliet.c b/fs/isofs/joliet.c
> index b1f4a105ee87..0832f40a9a2b 100644
> --- a/fs/isofs/joliet.c
> +++ b/fs/isofs/joliet.c
> @@ -15,19 +15,26 @@
>   * Convert Unicode 16 to UTF-8 or ASCII.
>   */
>  static int
> -uni16_to_x8(unsigned char *ascii, __be16 *uni, int len, struct nls_table *nls)
> +uni16_to_x8(unsigned char *ascii, __be16 *uni, int len, struct nls_table *nls,
> +	    int outsize)
>  {
>  	__be16 *ip, ch;
> -	unsigned char *op;
> +	unsigned char *op, *end;
>  
>  	ip = uni;
>  	op = ascii;
> +	end = ascii + outsize - 1;	/* leave room for the terminator */
>  
>  	while ((ch = get_unaligned(ip)) && len) {
>  		int llen;
> -		llen = nls->uni2char(be16_to_cpu(ch), op, NLS_MAX_CHARSET_SIZE);
> +
> +		if (op >= end)
> +			break;
> +		llen = nls->uni2char(be16_to_cpu(ch), op, end - op);
>  		if (llen > 0)
>  			op += llen;
> +		else if (llen == -ENAMETOOLONG)
> +			break;
>  		else
>  			*op++ = '?';
>  		ip++;
> @@ -59,7 +66,8 @@ get_joliet_filename(struct iso_directory_record * de, unsigned char *outname, st
>  				outname, ISOFS_NAME_BUF_SIZE);
>  	} else {
>  		len = uni16_to_x8(outname, (__be16 *) de->name,
> -				de->name_len[0] >> 1, nls);
> +				de->name_len[0] >> 1, nls,
> +				ISOFS_NAME_BUF_SIZE);
>  	}
>  	if ((len > 2) && (outname[len-2] == ';') && (outname[len-1] == '1'))
>  		len -= 2;
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR