Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 8 May 2020 00:27:46 -0700
From:   Kees Cook <keescook@chromium.org>
To:     WeiXiong Liao <liaoweixiong@allwinnertech.com>
Cc:     Anton Vorontsov <anton@enomsg.org>,
        Colin Cross <ccross@android.com>,
        Tony Luck <tony.luck@intel.com>,
        Petr Mladek <pmladek@suse.com>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        linux-doc@vger.kernel.org, linux-mtd@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 00/12] pstore: mtd: support crash log to block and mtd
 device
Message-ID: <202005080020.41C33738@keescook>
References: <20200508064004.57898-1-keescook@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200508064004.57898-1-keescook@chromium.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 07, 2020 at 11:39:52PM -0700, Kees Cook wrote:
> So far, I've identified the following stuff left to do:
> [...]
>         - implement ramoops-like probe feature for pstore/blk

With the following hack, I'm able to start testing the series:

diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index a736555e1ed3..7145da079267 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -373,12 +373,14 @@ int psblk_register_blkdev(unsigned int major, unsigned int flags,
 	if (IS_ERR(binfo))
 		return PTR_ERR(binfo);
 
+#if 0
 	/* only allow driver matching the @blkdev */
 	if (!binfo->devt || MAJOR(binfo->devt) != major) {
 		pr_debug("invalid major %u (expect %u)\n",
 				major, MAJOR(binfo->devt));
 		return -ENODEV;
 	}
+#endif
 
 	/* hold bdev exclusively */
 	bdev = psblk_get_bdev(holder);
@@ -423,7 +425,7 @@ void psblk_unregister_blkdev(unsigned int major)
 	struct psblk_device dev = {.read = psblk_generic_blk_read};
 	void *holder = blkdev;
 
-	if (psblk_bdev && MAJOR(psblk_bdev->bd_dev) == major) {
+	if (psblk_bdev/* && MAJOR(psblk_bdev->bd_dev) == major*/) {
 		psblk_unregister_device(&dev);
 		psblk_put_bdev(psblk_bdev, holder);
 		blkdev_panic_write = NULL;
@@ -476,6 +478,24 @@ int pstore_blk_usr_info(struct pstore_blk_info *info)
 }
 EXPORT_SYMBOL_GPL(pstore_blk_usr_info);
 
+static int __init pstore_blk_init(void)
+{
+	int ret = 0;
+
+	if (blkdev[0])
+		ret = psblk_register_blkdev(0, 0, NULL);
+
+	return ret;
+}
+postcore_initcall(pstore_blk_init);
+
+static void __exit pstore_blk_exit(void)
+{
+	psblk_unregister_blkdev(0);
+}
+module_exit(pstore_blk_exit);
+
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("WeiXiong Liao <liaoweixiong@allwinnertech.com>");
 MODULE_DESCRIPTION("pstore backend for block devices");


Then I can get things up and running with:

# insmod pstore.ko compress=off
# insmod pstore_zone.ko
# truncate pstore-blk.raw --size 100M
# losetup -f --show pstore-blk.raw
/dev/loop0
# insmod pstore_blk.ko blkdev=/dev/loop0 kmsg_size=16 console_size=64

So far, I've hit a few bugs. The most obvious is that "rmmod" causes a
fault, so I think locking and other things need to be fixed up further.
After that, it looked like all the compressed files were failing to
decompress, which implies some kind of buffer offset problem. When I
loaded with pstore.compress=off I got readable logs, but there is a span
of garbage between the header and the body in
/sys/fs/pstore/dmesg-pstore-zone-1 etc.

Cool so far! It just needs a bit more testing a polish. :)

-- 
Kees Cook