linux-ext4 - Re: [PATCH 05/11] e2fsck: Fix entries with invalid encoded characters

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201106182258.GB79496@sol.localdomain>
Date:   Fri, 6 Nov 2020 10:22:58 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Arnaud Ferraris <arnaud.ferraris@...labora.com>
Cc:     linux-ext4@...r.kernel.org,
        Gabriel Krisman Bertazi <krisman@...labora.com>
Subject: Re: [PATCH 05/11] e2fsck: Fix entries with invalid encoded characters

On Thu, Nov 05, 2020 at 05:16:37PM +0100, Arnaud Ferraris wrote:
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 8eecd958..968734e8 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c

Theres a comment at the top of this file that could be updated to mention
inode_casefold_map.

> +static int encoded_check_name(e2fsck_t ctx,
> +			      struct ext2_dir_entry *dirent,
> +			      struct problem_context *pctx)
> +{
> +	const struct ext2fs_nls_table *tbl = ctx->fs->encoding;
> +	int ret;
> +	int len = ext2fs_dirent_name_len(dirent);
> +	char *pos, *end;
> +
> +	ret = ext2fs_check_encoded_name(tbl, dirent->name, len, &pos);
> +	if (ret < 0) {
> +		fatal_error(ctx, _("NLS is broken."));
> +	} else if(ret > 0) {
> +		ret = fix_problem(ctx, PR_2_BAD_NAME, pctx);
> +		if (ret) {
> +			end = &dirent->name[len];
> +			for (; *pos && pos != end; pos++)
> +				*pos = '.';
> +		}
> +	}
> +
> +	return (ret || check_name(ctx, dirent, pctx));
> +}

Maybe this should use a new problem code instead of reusing PR_2_BAD_NAME?

> @@ -998,11 +1028,18 @@ static int check_dir_block(ext2_filsys fs,
>  	size_t	max_block_size;
>  	int	hash_flags = 0;
>  	static char *eop_read_dirblock = NULL;
> +	int cf_dir = 0;
>  
>  	cd = (struct check_dir_struct *) priv_data;
>  	ibuf = buf = cd->buf;
>  	ctx = cd->ctx;
>  
> +	/* We only want filename encoding verification on strict
> +	 * mode. */
> +	if (ext2fs_test_inode_bitmap2(ctx->inode_casefold_map, ino) &&
> +	    (ctx->fs->super->s_encoding_flags & EXT4_ENC_STRICT_MODE_FL))
> +		cf_dir = 1;
> +
>  	if (ctx->flags & E2F_FLAG_RUN_RETURN)
>  		return DIRENT_ABORT;
>  
> @@ -1483,7 +1520,11 @@ skip_checksum:
>  		if (check_filetype(ctx, dirent, ino, &cd->pctx))
>  			dir_modified++;
>  
> -		if (dir_encpolicy_id == NO_ENCRYPTION_POLICY) {
> +		if (cf_dir) {
> +			/* casefolded directory */
> +			if (encoded_check_name(ctx, dirent, &cd->pctx))
> +				dir_modified++;
> +		} else if (dir_encpolicy_id == NO_ENCRYPTION_POLICY) {
>  			/* Unencrypted directory */
>  			if (check_name(ctx, dirent, &cd->pctx))
>  				dir_modified++;

It might be a good idea to rearrange this logic to be ready for directories that
are both encrypted and casefolded, where checking the encoded names won't be
possible:

	if (dir_encpolicy_id != NO_ENCRYPTION_POLICY) {
		/* handle encrypted dir */
	} else if (cf_dir) {
		/* handle casefolded dir */
	} else {
		/* handle regular dir */
	}