linux-kernel - RE: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software

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: <5313baaad14c40d09738bf63e4659ac9@paragon-software.com>
Date:   Fri, 30 Oct 2020 15:51:10 +0000
From:   Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To:     Pali Rohár <pali@...nel.org>
CC:     "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dsterba@...e.cz" <dsterba@...e.cz>,
        "aaptel@...e.com" <aaptel@...e.com>,
        "willy@...radead.org" <willy@...radead.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "joe@...ches.com" <joe@...ches.com>,
        "mark@...mstone.com" <mark@...mstone.com>,
        "nborisov@...e.com" <nborisov@...e.com>,
        "linux-ntfs-dev@...ts.sourceforge.net" 
        <linux-ntfs-dev@...ts.sourceforge.net>,
        "anton@...era.com" <anton@...era.com>
Subject: RE: [PATCH v11 00/10] NTFS read-write driver GPL implementation by
 Paragon Software

From: Pali Rohár <pali@...nel.org>
Sent: Friday, October 30, 2020 6:25 PM
> To: Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
> Cc: linux-fsdevel@...r.kernel.org; viro@...iv.linux.org.uk; linux-kernel@...r.kernel.org; dsterba@...e.cz; aaptel@...e.com;
> willy@...radead.org; rdunlap@...radead.org; joe@...ches.com; mark@...mstone.com; nborisov@...e.com; linux-ntfs-
> dev@...ts.sourceforge.net; anton@...era.com
> Subject: Re: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software
> 
> Hello and thanks for update!
> 
> I have just two comments for the last v11 version.
> 
> I really do not like nls_alt mount option and I do not think we should
> merge this mount option into ntfs kernel driver. Details I described in:
> https://lore.kernel.org/linux-fsdevel/20201009154734.andv4es3azkkskm5@pali/
> 
> tl;dr it is not systematic solution and is incompatible with existing
> in-kernel ntfs driver, also incompatible with in-kernel vfat, udf and
> ext4 (with UNICODE support) drivers. In my opinion, all kernel fs
> drivers which deals with UNICODE should handle it in similar way.
> 

Hello Pali! First of all, apologies for not providing a feedback on your previous
message regarding the 'nls_alt'. We had internal discussions on the topic and
overall conclusion is that: we do not want to compromise Kernel standards with
our submission. So we will remove the 'nls_alt' option in the next version.

However, there are still few points we have on the topic, please read below.

> It would be really bad if userspace application need to behave
> differently for this new ntfs driver and differently for all other
> UNICODE drivers.
> 

The option does not anyhow affect userspace applications. For the "default" example
of unzip/tar:
1 - if this option is not applied (e.g. "vfat case"), trying to unzip an archive with, e.g. CP-1251,
names inside to the target fs volume, will return error, and issued file(s) won't be unzipped;
2 - if this option is applied and "nls_alt" is set, the above case will result in unzipping all the files;

Also, this issue in general only applies to "non-native" filesystems. I.e. ext4 is not affected by it
in any case, as it just stores the name as bytes, no matter what those bytes are. The above case
won't give an unzip error on ext4. The only symptom of this would be, maybe, "incorrect encoding"
marking within the listing of such files (in File Manager or Terminal, e.g. in Ubuntu), but there won't
be an unzip process termination with incomplete unarchived fileset, unlike it is for vfat/exfat/ntfs
without "nls_alt".

> Second comment is simplification of usage nls_load() with UTF-8 parameter
> which I described in older email:
> https://lore.kernel.org/linux-fsdevel/948ac894450d494ea15496c2e5b8c906@paragon-software.com/
> 
> You wrote that you have applied it, but seems it was lost (maybe during
> rebase?) as it is not present in the last v11 version.
> 
> I suggested to not use nls_load() with UTF-8 at all. Your version of
> ntfs driver does not use kernel's nls utf8 module for UTF-8 support, so
> trying to load it should be avoided. Also kernel can be compiled without
> utf8 nls module (which is moreover broken) and with my above suggestion,
> ntfs driver would work correctly. Without that suggestion, mounting
> would fail.

Thanks for pointing that out. It is likely the "nls_load()" fixes were lost during rebase.
Will recheck it and return them to the v12.

Best regards!