linux-kernel - Re: [PATCH 06/10] AXFS: axfs

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] [day] [month] [year] [list]

Date:	Wed, 29 Oct 2008 14:59:46 +0100 (CET)
From:	Geert Uytterhoeven <Geert.Uytterhoeven@...ycom.com>
To:	Jared Hulbert <jaredeh@...il.com>
cc:	Linux-kernel@...r.kernel.org, linux-embedded@...r.kernel.org,
	linux-mtd <linux-mtd@...ts.infradead.org>,
	Jörn Engel <joern@...fs.org>,
	tim.bird@...SONY.COM, cotte@...ibm.com, nickpiggin@...oo.com.au
Subject: Re: [PATCH 06/10] AXFS: axfs_super.c

On Wed, 20 Aug 2008, Jared Hulbert wrote:
> --- /dev/null
> +++ b/fs/axfs/axfs_super.c

> +static int axfs_fill_region_data(struct super_block *sb,
> +				 struct axfs_region_desc *region, int force)
> +{
> +	struct axfs_super *sbi = AXFS_SB(sb);
> +	unsigned long addr;
> +	void *buff = NULL;
> +	void *vaddr;
> +	int err = -ENOMEM;
> +	u64 size = region->size;
> +	u64 fsoffset = region->fsoffset;
> +	u64 end = fsoffset + size;
> +	u64 c_size = region->compressed_size;
> +
> +	if (size == 0)
> +		return 0;
> +
> +	if (AXFS_IS_REGION_INCORE(region))
> +		goto incore;
> +
> +	if (AXFS_IS_REGION_COMPRESSED(region))
> +		goto incore;
> +
> +	if (AXFS_IS_REGION_XIP(sbi, region)) {
> +		if ((end > sbi->mmap_size) && (force))
> +			goto incore;
> +		addr = sbi->virt_start_addr;
> +		addr += (unsigned long)fsoffset;
> +		region->virt_addr = (void *)addr;
> +		return 0;
> +	}
> +
> +	if (force)
> +		goto incore;
> +
> +	region->virt_addr = NULL;
> +	return 0;
> +
> +incore:
> +	region->virt_addr = vmalloc(size);
> +	if (!region->virt_addr)
> +		goto out;
> +	vaddr = region->virt_addr;
> +
> +	if (AXFS_IS_REGION_COMPRESSED(region)) {
> +		buff = vmalloc(c_size);
> +		if (!buff)
> +			goto out;
> +		axfs_copy_metadata(sb, buff, fsoffset, c_size);
> +		err = axfs_uncompress_block(vaddr, size, buff, c_size);
                ^^^
axfs_uncompress_block() doesn't return an error code, so `err' is a bit of a
misnomer.

> +		if (!err)
                    ^^^^
Instead, it returns zero for error, hence this check is correct...

> +			goto out;
> +		vfree(buff);
> +	} else {
> +		axfs_copy_metadata(sb, vaddr, fsoffset, size);
> +	}
> +
> +	return 0;
> +
> +out:
> +	if (buff)
> +		vfree(buff);
> +	if (region->virt_addr)
> +		vfree(region->virt_addr);
> +	return err;
               ^^^
... but it will also return zero here, which is not correct.

Probably it's better to let axfs_uncompress_block() return an (negative) error
code (e.g. -EINVAL or -EIO) in case of failure, which you can just let
propagate.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@...ycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010