Re: [PATCH] Two bugs in fs/isofs

From: Jan Kara
Date: Wed Oct 21 2015 - 08:51:46 EST


Hi,

thanks for detailed reports. For now I did some research on the case of
file name truncation.

> ===============================================================
> "fs/isofs/rock.c coarsely truncates file names of 254 or 255 bytes length"
> ---------------------------------------------------------------
>
> Reproduce by:
>
> # Create Rock Ridge file name of 254 bytes length.
> # File "/bin/true" is assumed to exist and be readable.
> # Else use any other readable file instead.
>
> genisoimage -R -o test.iso -graft-points /12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234=/bin/true
>
> mount -o loop test.iso /mnt/iso
>
> ls /mnt/iso
>
> shows
>
> 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456
>
> That's 146 characters only.
> If you do the same with xorriso you get other truncation lengths.
>
> --------------- Source analysis:
>
> There is a deliberate limit of < 254 in fs/isofs/rock.c
>
> if ((strlen(retname) + rr->len - 5) >= 254) {
> truncate = 1;
> break;
> }
>
> It is not clear to me why there should be such a limit.
> In the Debian bug report i discuss my findings about clients of
> the Rock Ridge name reader.
> Length up to 255 should be ok.
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=798300

Looking into the code, we should be fine with names upto 1023 characters
long (that's the buffer space we have available in that code). However I
guess there's no point in supporting more than NAME_MAX (255). So I agree
with you here the limit of 254 seems to be off by two. I've tried to dig in
history but that code seems to predate even BitKeeper times...

> The truncation drops the whole second NM entry which contains
> the rest of the file name bytes. In above example it contained
> the 108 missing digits.

Yeah, that should be reasonably easy to fix.

> Truncation nowadays has to take into respect that UTF-8 may
> consist of multiple bytes and should avoid to leave incomplete
> byte sequences.
> (Does the kernel have a function for this ?)

Well, such truncation function would have to be specific to encoding the fs
uses.

> The truncated names are not necessarily unique within the
> directory. (There is few chance to check this when the name
> gets composed from NM entries.)

Well, true but is it worth the bother? I mean realistically, do people use
media with more than 255 characters in a file name or is it mostly a
theoretical concern?

Honza

> --------------- Remedy proposal:
>
> I implemented truncation in libisofs this way (from man xorriso):
>
> Path components which are longer than the given
> number [64 to 255] will get truncated and have their last 33 bytes
> overwritten by a colon ':' and the hex representation of the MD5
> of the first 4095 bytes of the whole oversized name. Potential
> incomplete UTF-8 characters will get their leading bytes
> replaced by '_'.
>
> This gives hope for unique truncated names and an opportunity for
> implementing lookup via the oversized untruncated name.
> The situation in libisofs gets a bit more complicated by the number
> being adjustable (for Linux kernels which will be old in future).
>
> If there is interest, i would try to port the truncation and the
> lookup algorithms from libisofs to Linux. But i guess that
> i am not the first one who needs to truncate UTF-8. So possibly
> there are better ways than my userland code.
>
> In my test kernel i only implemented and tested protection of the
> innocent for now:
>
> * Allow Rock Ridge names of 254 and 255 bytes length.
>
> --- linux-source-4.1/fs/isofs/rock.c 2015-08-17 05:52:51.000000000 +0200
> +++ linux-4.1.6/fs/isofs/rock.c 2015-10-07 21:53:13.281654707 +0200
> @@ -267,7 +267,7 @@ repeat:
> rr->u.NM.flags);
> break;
> }
> - if ((strlen(retname) + rr->len - 5) >= 254) {
> + if ((strlen(retname) + rr->len - 5) >= 256) {
> truncate = 1;
> break;
> }
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/