linux-kernel - Re: [PATCH v6 01/10] libfs: Create the helper function generic_ci_validate_strict

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: <87bjzls6ff.fsf@mailhost.krisman.be>
Date: Tue, 15 Oct 2024 11:59:48 -0400
From: Gabriel Krisman Bertazi <gabriel@...sman.be>
To: André Almeida <andrealmeid@...lia.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,  Christian Brauner
 <brauner@...nel.org>,  Jan Kara <jack@...e.cz>,  Theodore Ts'o
 <tytso@....edu>,  Andreas Dilger <adilger.kernel@...ger.ca>,  Hugh Dickins
 <hughd@...gle.com>,  Andrew Morton <akpm@...ux-foundation.org>,  Jonathan
 Corbet <corbet@....net>,  smcv@...labora.com,  kernel-dev@...lia.com,
  linux-fsdevel@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-ext4@...r.kernel.org,  linux-mm@...ck.org,
  linux-doc@...r.kernel.org
Subject: Re: [PATCH v6 01/10] libfs: Create the helper function
 generic_ci_validate_strict_name()

André Almeida <andrealmeid@...lia.com> writes:

> +static inline bool generic_ci_validate_strict_name(struct inode *dir, struct qstr *name)
> +{
> +	if (!IS_CASEFOLDED(dir) || !sb_has_strict_encoding(dir->i_sb))
> +		return true;
> +
> +	/*
> +	 * A casefold dir must have a encoding set, unless the filesystem
> +	 * is corrupted
> +	 */
> +	if (WARN_ON_ONCE(!dir->i_sb->s_encoding))
> +		return true;
> +
> +	return utf8_validate(dir->i_sb->s_encoding, name);

There is something fishy here.  Concerningly, the fstests test doesn't
catch it.

utf8_validate is defined as:

  int utf8_validate(const struct unicode_map *um, const struct qstr *str)

Which returns 0 on success and !0 on error. Thus, when casting to bool,
the return code should be negated.

But generic/556 doesn't fail. That's because we are over cautious, and
also check the string at the end of generic_ci_d_hash.  So we never
really reach utf8_validate in the tested case.

But if you comment the final if in generic_ci_d_hash, you'll see this
patchset regresses the fstests case generic/556 over ext4.

We really need the check in both places, though.  We don't want to rely
on the behavior of generic_ci_d_hash to block invalid filenames, as that
might change.

-- 
Gabriel Krisman Bertazi