LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: marcin.slusarz@gmail.com
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 02/10] udf: fix udf_build_ustr
Date: Thu, 31 Jan 2008 11:45:32 +0100	[thread overview]
Message-ID: <20080131104532.GB1461@duck.suse.cz> (raw)
In-Reply-To: <1201727040-6769-3-git-send-email-marcin.slusarz@gmail.com>

On Wed 30-01-08 22:03:52, marcin.slusarz@gmail.com wrote:
> udf_build_ustr was completely broken when
> size >= UDF_NAME_LEN - 1 or size < 2
> 
> nobody noticed because all callers set size
> to acceptable values (constants)
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Jan Kara <jack@suse.cz>
> ---
>  fs/udf/unicode.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index f969617..f4e54e5 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -47,16 +47,16 @@ static int udf_char_to_ustr(struct ustr *dest, const uint8_t *src, int strlen)
>   */
>  int udf_build_ustr(struct ustr *dest, dstring *ptr, int size)
>  {
> -	int usesize;
> +	u8 usesize;
  What is the purpose for this? Why not just leave int there? Arithmetics
is usually best done in ints if that's possible...
>  
> -	if ((!dest) || (!ptr) || (!size))
> +	if (!dest || !ptr || size < 2)
>  		return -1;
>  
> -	memset(dest, 0, sizeof(struct ustr));
> -	usesize = (size > UDF_NAME_LEN) ? UDF_NAME_LEN : size;
> +	usesize = min_t(size_t, size - 2, sizeof(dest->u_name));
>  	dest->u_cmpID = ptr[0];
> -	dest->u_len = ptr[size - 1];
> -	memcpy(dest->u_name, ptr + 1, usesize - 1);
> +	dest->u_len = usesize;
> +	memcpy(dest->u_name, ptr + 1, usesize);
> +	memset(dest->u_name + usesize, 0, sizeof(dest->u_name) - usesize);
  Hmm, after parsing what the standard says (ugh), I don't think the code is
wrong (at least I think you made it incorrect). The caller of
udf_char_to_ustr() specifies length of the field (not length of the
string). Now, in the last character of the field is stored the number of
characters in the string and in the first character of the field is stored
encoding of the string. So the original code seems correct.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2008-01-31 10:45 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-30 21:03 [PATCH 00/10] udf: cleanups marcin.slusarz
2008-01-30 21:03 ` [PATCH 01/10] udf: udf_CS0toUTF8 cleanup marcin.slusarz
2008-01-31  9:57   ` Jan Kara
2008-01-30 21:03 ` [PATCH 02/10] udf: fix udf_build_ustr marcin.slusarz
2008-01-31 10:45   ` Jan Kara [this message]
2008-01-31 19:57     ` Marcin Slusarz
2008-02-04 19:31       ` Jan Kara
2008-02-04 21:27         ` Marcin Slusarz
2008-02-05 15:29           ` Jan Kara
2008-02-05 19:17             ` Marcin Slusarz
2008-01-30 21:03 ` [PATCH 03/10] udf: udf_CS0toNLS cleanup marcin.slusarz
2008-01-31 12:23   ` Jan Kara
2008-01-30 21:03 ` [PATCH 04/10] udf: constify crc marcin.slusarz
2008-01-31 12:58   ` Jan Kara
2008-01-30 21:03 ` [PATCH 05/10] udf: simple cleanup of truncate.c marcin.slusarz
2008-01-31 13:01   ` Jan Kara
2008-01-30 21:03 ` [PATCH 06/10] udf: truncate: create function for updating of Allocation Ext Descriptor marcin.slusarz
2008-01-31 17:08   ` Jan Kara
2008-01-31 18:18     ` Marcin Slusarz
2008-01-30 21:03 ` [PATCH 07/10] udf: replace all adds to little endians variables with le*_add_cpu marcin.slusarz
2008-01-31 16:42   ` Jan Kara
2008-01-30 21:03 ` [PATCH 08/10] udf: simplify __udf_read_inode marcin.slusarz
2008-01-31 16:46   ` Jan Kara
2008-01-30 21:03 ` [PATCH 09/10] udf: replace udf_*_offset macros with functions marcin.slusarz
2008-01-31 16:48   ` Jan Kara
2008-01-30 21:04 ` [PATCH 10/10] udf: constify udf_bitmap_lookup array marcin.slusarz
2008-01-31 16:52   ` Jan Kara
2008-02-02 21:37     ` Marcin Slusarz
2008-02-04 19:15       ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080131104532.GB1461@duck.suse.cz \
    --to=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcin.slusarz@gmail.com \
    --subject='Re: [PATCH 02/10] udf: fix udf_build_ustr' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).