linux-kernel - RE: [PATCH v2 6/7] udf: Remove struct ustr as non-needed intermediate storage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <000001d14c74$679154b0$36b3fe10$@mentor.com>
Date:	Mon, 11 Jan 2016 16:31:40 +0300
From:	Andrew Gabbasov <andrew_gabbasov@...tor.com>
To:	'Jan Kara' <jack@...e.cz>
CC:	Jan Kara <jack@...e.com>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 6/7] udf: Remove struct ustr as non-needed intermediate storage

Hi Jan,

Thank you for your review!
And sorry for a delay with response, we had several days of holidays.

> -----Original Message-----
> From: Jan Kara [mailto:jack@...e.cz] 
> Sent: Monday, January 04, 2016 3:33 PM
> To: Gabbasov, Andrew
> Cc: Jan Kara; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v2 6/7] udf: Remove struct ustr as non-needed
intermediate storage

[skipped]

> >  
> > -	ocu[length - 1] = (uint8_t)u_len + 1;
> > -	return u_len + 1;
> > +	return u_len;
>
> It seems you removed setting of the length in the resulting CS0 string.

Yes, and it was done deliberately.
udf_name_to_CS0 and its caller udf_put_filename functions are used for
writing File Identifier fields only, which are not "dstrings", that is
not containing the length in last byte. The last byte with the length
from these functions would not be copied to filesystem or used in any
other way.

[skipped]

> > -	ret = udf_translate_to_linux(dname, dlen,
> > -				     filename->u_name, filename->u_len,
> > -				     unifilename->u_name,
unifilename->u_len);
> > +	ret = udf_translate_to_linux(dname, dlen, filename, dlen, sname,
slen);
>
> Umm, but here you pass CS0 name to udf_translate_to_linux() including
> beginning encoding character which didn't happen before your patch.
> So in case we have to compute CRC it will be different.

Yes, you are correct, this is a mistake (introduced when splitting
the original single patch). I will correct it in version 3 of the patches
(I will use sname + 1 and slen - 1).
Thank you for catching it!

I will prepare an update soon.

Thanks.

Best regards,
Andrew