linux-kernel - Re: RFC: direct MTD support for SquashFS

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: <87wrx32rvn.fsf@tac.ki.iif.hu>
Date:	Tue, 23 Mar 2010 12:34:52 +0100
From:	Ferenc Wagner <wferi@...f.hu>
To:	Phillip Lougher <phillip.lougher@...il.com>
Cc:	Phillip Lougher <phillip@...gher.demon.co.uk>,
	linux-fsdevel@...r.kernel.org, linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org, linux-embedded@...r.kernel.org
Subject: Re: RFC: direct MTD support for SquashFS

Phillip Lougher <phillip.lougher@...il.com> writes:

> A couple of specific comments...
>
> +/* A backend is initialized for each SquashFS block read operation,
> + * making further sequential reads possible from the block.
> + */
> +static void *bdev_init(struct squashfs_sb_info *msblk, u64 index,
> size_t length)
> +{
> +	struct squashfs_bdev *bdev = msblk->backend_data;
> +	struct buffer_head *bh;
> +
> +	bh = kcalloc((msblk->block_size >> bdev->devblksize_log2) + 1,
> +			sizeof(*bh), GFP_KERNEL);
>
> You should alloc against the larger of msblk->block_size and
> METADATA_SIZE (8 Kbytes).  Block_size could be 4 Kbytes only.

I plugged in a max().  Couldn't that trailing +1 be converted into a +2
like this?

bh = kcalloc((max(msblk->block_size, METADATA_SIZE) + 2) >> bdev->devblksize_log2

> +static int fill_bdev_super(struct super_block *sb, void *data, int silent)
>
> This function looks rather 'back-to-front' to me.  I'm assuming that
> squashfs_fill_super2() will be the current fill superblock function?
> This function wants to read data off the filesystem through the
> backend, and yet the backend (bdev, mblk->backend_data) hasn't been
> initialised when it's called...

I solved it by introducing a callback function for adding the backend.
That may be overkill, but it seems to give the most shared code.

The attached patch series survived some testing here.  My only doubt:
the current backend interface necessitates a memory copy from the buffer
heads.  This is no problem for mtd and lzma which copy the data anyway,
but makes this code less efficient in the bdev+zlib case.

I've got one more patch, which I forgot to export, to pull out the
common logic from the backend init functions back into squashfs_read_data().
With the bdev backend, that entails reading the first block twice in a
row most of the time.  This again could be worked around by extending
the backend interface, but I'm not sure if it's worth it.

How does this look like now?
-- 
Regards,
Feri.

View attachment "squashfs-mtd.mbox" of type "text/x-diff" (45783 bytes)