linux-kernel - Re: [RFC PATCH 1/4] fs/ntfs3: Use new api for mounting

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: <20210816132416.zk326rbhwe7eaj3i@pali>
Date:   Mon, 16 Aug 2021 15:24:16 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Kari Argillander <kari.argillander@...il.com>
Cc:     Christoph Hellwig <hch@....de>,
        Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        ntfs3@...ts.linux.dev, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, Matthew Wilcox <willy@...radead.org>
Subject: Re: [RFC PATCH 1/4] fs/ntfs3: Use new api for mounting

On Monday 16 August 2021 16:14:17 Kari Argillander wrote:
> > > + * Load nls table or if @nls is utf8 then return NULL because
> > > + * nls=utf8 is totally broken.
> > > + */
> > > +static struct nls_table *ntfs_load_nls(char *nls)
> > > +{
> > > +	struct nls_table *ret;
> > > +
> > > +	if (!nls)
> > > +		return ERR_PTR(-EINVAL);
> > > +	if (strcmp(nls, "utf8"))
> > > +		return NULL;
> > > +	if (strcmp(nls, CONFIG_NLS_DEFAULT))
> > > +		return load_nls_default();
> > > +
> > > +	ret = load_nls(nls);
> > > +	if (!ret)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	return ret;
> > > +}
> > 
> > This looks like something quite generic and not file system specific.
> > But I haven't found time to look at the series from Pali how this all
> > fits together.
> 
> It is quite generic I agree. Pali's series not implemeted any new way
> doing this thing. In many cases Pali uses just load_nls and not
> load_nls_default. This function basically use that if possible. It seems
> that load_nls_default does not need error path so that's why it is nicer
> to use.

Yes, I'm using what is currently available. But providing some helper
function should be a nice cleanup.

> One though is to implement api function load_nls_or_utf8(). Then we do not
> need to test this utf8 stuff in all places.

Beware that there are more cases which can happen:

- iocharset is not specified
  --> then driver default behavior is used
      --> it is either some fixed encoding (e.g. iso8859-1, utf8) or
          CONFIG_NLS_DEFAULT (*); so it should behave like iocharset is
          set to that fixed encoding or CONFIG_NLS_DEFAULT
- iocharset is set to utf8
  --> then native utf8* functions should be used instead of nls
- iocharset is set to CONFIG_NLS_DEFAULT
  --> then load_nls_default() should be used which is IIRC guaranteed to
      not fail
- iocharset is not set to utf8, neither to CONFIG_NLS_DEFAULT
  --> then load_nls(iocharset) should be used; this may fail

(*) - it is pity that not all fs drivers are using CONFIG_NLS_DEFAULT
      and some are using some their own fixed encoding... it just
      increase mess and "user surprise"