linux-kernel - Re: [PATCH] Two bugs in fs/isofs

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] [day] [month] [year] [list]
Message-ID: <20151021125138.GA8856@quack.suse.cz>
Date:	Wed, 21 Oct 2015 14:51:38 +0200
From:	Jan Kara <jack@...e.cz>
To:	Thomas Schmitt <scdbackup@....net>
Cc:	linux-kernel@...r.kernel.org, viro@...iv.linux.org.uk, jack@...e.cz
Subject: Re: [PATCH] Two bugs in fs/isofs

  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@...e.com>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/