linux-ext4 - Re: ext3: ext4: Using uninitialized value

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: <4CB5DAC3.3040605@redhat.com>
Date:	Wed, 13 Oct 2010 11:13:55 -0500
From:	Eric Sandeen <sandeen@...hat.com>
To:	Roman Borisov <ext-roman.borisov@...ia.com>
CC:	linux-ext4@...r.kernel.org
Subject: Re: ext3: ext4: Using uninitialized value

On 10/13/2010 09:40 AM, Roman Borisov wrote:
> Hello,
> 
> Could you clarify is there a bug in fs/ext4/namei.c,
> ext4_dx_find_entry() and fs/ext4/namei.c, ext3_dx_find_entry()?

that was introduced with:

commit acfa1823d33859b0db77701726c9ca5ccc6e6f25
Author: Andreas Dilger <adilger@...sterfs.com>
Date:   Thu Jun 23 00:09:45 2005 -0700

    [PATCH] Support for dx directories in ext3_get_parent (NFSD)

so maybe Andreas knows offhand ;)  but I think:

> <code>
> static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
> ...
>         if (namelen > 2 || name[0] != '.'|| (namelen == 2 && name[1] != '.')) {

This is a fancy way of saying name is not "." or ".."

>                 if (!(frame = dx_probe(entry, dir, &hinfo, frames, err)))
>                         return NULL;
>         } else {

so here it -is- "." or ".." -

>                 frame = frames;
>                 frame->bh = NULL;                       /* for dx_release() */
>                 frame->at = (struct dx_entry *)frames;  /* hack for zero entry*/
>                 dx_set_block(frame->at, 0);             /* dx_root block is 0 */

now frame->at.block = 0

>         }
>         hash = hinfo.hash;
>         do {
>                 block = dx_get_block(frame->at);

block = 0 (we just put it there)

>                 if (!(bh = ext3_bread (NULL,dir, block, 0, err)))
>                         goto errout;

so we look up block 0 in the dir inode

>                 de = (struct ext3_dir_entry_2 *) bh->b_data;
>                 top = (struct ext3_dir_entry_2 *) ((char *) de + sb->s_blocksize -
>                                        EXT3_DIR_REC_LEN(0));

and get to the dir entry, and search through them for our name

>                 for (; de < top; de = ext3_next_entry(de)) {
>                         int off = (block << EXT3_BLOCK_SIZE_BITS(sb))
>                                   + ((char *) de - bh->b_data);
> 
>                         if (!ext3_check_dir_entry(__func__, dir, de, bh, off)) {
>                                 brelse(bh);
>                                 *err = ERR_BAD_DX_DIR;
>                                 goto errout;
>                         }
> 
>                         if (ext3_match(namelen, name, de)) {

here we should find the . or .. (it's always going to be there, right?)

>                                 *res_dir = de;
>                                 dx_release(frames);
>                                 return bh;

so we return

>                         }
>                 }

before we get here:

> ...
>                 retval = ext3_htree_next_block(dir, hash, frame,
>                                                frames, NULL);
> ...
> </code>

-Eric

> In the code above: hinfo.hash is not initialized in "else" case.
> Should it be initialized as NULL?
> Or maybe implementation doesn't assume to call ext3_htree_next_block()
> in such case?
> 
> Thanks,
> Roman
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html