On Wed 30-01-08 22:03:52, [EMAIL PROTECTED] 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 <[EMAIL PROTECTED]>
> Cc: Jan Kara <[EMAIL PROTECTED]>
> ---
> 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 <[EMAIL PROTECTED]>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
