linux-kernel - Re: [PATCH 00/11] Add compression support to pstore

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]

Date:	Mon, 05 Aug 2013 22:40:35 +0530
From:	Aruna Balakrishnaiah <aruna@...ux.vnet.ibm.com>
To:	Tony Luck <tony.luck@...il.com>
CC:	"linuxppc-dev@...abs.org" <linuxppc-dev@...abs.org>,
	"paulus@...ba.org" <paulus@...ba.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
	"keescook@...omium.org" <keescook@...omium.org>
Subject: Re: [PATCH 00/11] Add compression support to pstore

Hi Tony,

Thank you very much for testing my patches.

On Saturday 03 August 2013 03:42 AM, Tony Luck wrote:
> A quick experiment to use your patchset - but with compression
> disabled by tweaking this line in pstore_dump():
>
>      zipped_len = -1; //zip_data(dst, hsize + len);
>
> turned out well. This kernel dumps uncompressed dmesg blobs into pstore
> and gets them back out again.  So it seems likely that the problems are
> someplace in the compression/decompression code.

A quick look on my code suggests that problem could be in this part
of code.

In pstore_dump:

          if (zipped_len < 0) {
                         dst = psinfo->buf;
                         hsize = sprintf(dst, "%s#%d Part%d\n",
                                         why, oopscount, part);
                         size = psinfo->bufsize - hsize;
                         dst += hsize;
                         compressed = false;

                         if (!kmsg_dump_get_buffer(dumper, true, dst,
size, &len))
                                 break;
                 } else {
                         compressed = true;
      --->               len = zipped_len;
                 }

I am returning zipped_len as the length of the compressed data (which also
has hsize compressed). So returning hsize + len in pstore_write callback
will be wrong. It should just have been zipped_len. This might be adding
junk characters.

Can you please replace this hunk with:

                 if (zipped_len < 0) {
                         pr_err("Compression failed\n");
                         dst = psinfo->buf;
                         hsize = sprintf(dst, "%s#%d Part%d\n",
                                         why, oopscount, part);
                         size = psinfo->bufsize - hsize;
                         dst += hsize;
                         compressed = false;

                         if (!kmsg_dump_get_buffer(dumper, true, dst,
size, &len))
                                 break;
                         total_len = hsize + len;
                 } else {
                         compressed = true;
                         total_len = zipped_len;
                 }

                 ret = psinfo->write(PSTORE_TYPE_DMESG, reason, &id, part,
                                     oopscount, compressed, total_len, psinfo);
                 if (ret == 0 && reason == KMSG_DUMP_OOPS && pstore_is_mounted())
                         pstore_new_entry = 1;

                 total += total_len;
                 part++;

With the above hunk, atleast I dont see junk characters at the end in power.

I apologise, since I do not have the suitable machine to test this I am
not able to reproduce the scenarios you are stating. I need your help
in testing this.

- Aruna

> -Tony
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@...ts.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

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