On Wednesday 07 August 2013 05:06 AM, Tony Luck wrote:
On Mon, Aug 5, 2013 at 2:20 PM, Tony Luck <tony.l...@gmail.com> wrote:
Still have problems booting if there are any compressed images in ERST
to be inflated.
So I took another look at this part of the code ... and saw a couple of issues:

         while ((size = psi->read(&id, &type, &count, &time, &buf, &compressed,
                                 psi)) > 0) {
                 if (compressed && (type == PSTORE_TYPE_DMESG)) {
                         big_buf_sz = (psinfo->bufsize * 100) / 45;
                         big_buf = allocate_buf_for_decompression(big_buf_sz);

                         if (big_buf || stream.workspace)
Did you mean "&&" here rather that "||"?

Yes right, it should be &&.

                                 unzipped_len = pstore_decompress(buf, big_buf,
                                                         size, big_buf_sz);
Need an "else" here to set unzipped_len to -1 (or set it to -1 down
at the bottom of the loop ready for next time around.
                         if (unzipped_len > 0) {
                                 buf = big_buf;
This sets us up for problems.  First, you just overwrote the address
of the buffer that psi->read allocated - so we have a memory leak. But
worse than that we now double free the same buffer below when we
kfree(buf) and then kfree(big_buf)
                                 size = unzipped_len;
                                 compressed = false;
                         } else {
                                 pr_err("pstore: decompression failed;"
                                         "returned %d\n", unzipped_len);
                                 compressed = true;
                         }
                 }
                 rc = pstore_mkfile(type, psi->name, id, count, buf,
                                   compressed, (size_t)size, time, psi);
                 kfree(buf);
                 kfree(stream.workspace);
                 kfree(big_buf);
                 buf = NULL;
                 stream.workspace = NULL;
                 big_buf = NULL;
                 if (rc && (rc != -EEXIST || !quiet))
                         failed++;
         }


See attached patch that fixes these - but the code still looks like it
could be cleaned up a bit more.

The patch looks right. I will clean it up. Does the issue still persist after 
this?

-Tony

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

Reply via email to