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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <bffbecbb1003181440kc78f4dbvd25d4e2ea17b52e5@mail.gmail.com>
Date:	Thu, 18 Mar 2010 21:40:45 +0000
From:	Phillip Lougher <phillip.lougher@...il.com>
To:	Ferenc Wagner <wferi@...f.hu>
Cc:	Phillip Lougher <phillip@...gher.demon.co.uk>,
	Peter Korsgaard <jacmet@...site.dk>,
	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

On Thu, Mar 18, 2010 at 4:38 PM, Ferenc Wagner <wferi@...f.hu> wrote:
>
> I could only compare apples to oranges before porting the patch to the
> LZMA variant.  So I refrain from that for a couple of days yet.  But
> meanwhile I started adding a pluggable backend framework to SquashFS,
> and would much appreciate some comments about the applicability of this
> idea.  The patch is (intended to be) a no-op, applies on top of current
> git (a3d3203e4bb40f253b1541e310dc0f9305be7c84).

This looks promising, making the backend pluggable (like the new
compressor framework) is far better and cleaner than scattering the
code full of #ifdef's.  Far better than the previous patch :-)

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.

+static int fill_bdev_super(struct super_block *sb, void *data, int silent)
+{
+	struct squashfs_sb_info *msblk;
+	struct squashfs_bdev *bdev;
+	int err = squashfs_fill_super2(sb, data, silent, &squashfs_bdev_ops);
+	if (err)
+		return err;
+
+	bdev = kzalloc(sizeof(*bdev), GFP_KERNEL);
+	if (!bdev)
+		return -ENOMEM;
+
+	bdev->devblksize = sb_min_blocksize(sb, BLOCK_SIZE);
+	bdev->devblksize_log2 = ffz(~bdev->devblksize);
+
+	msblk = sb->s_fs_info;
+	msblk->backend_data = bdev;
+	return 0;
+}

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...

Phillip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/