All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Nitin Gupta <ngupta@vflare.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Fengguang Wu <fengguang.wu@intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] zram: factor-out zram_decompress_page() function (v2)
Date: Mon, 29 Oct 2012 21:57:08 +0300	[thread overview]
Message-ID: <20121029185708.GA2336@swordfish> (raw)
In-Reply-To: <CAPkvG_eZObsD=uCRGNygsrTs=ymjoA09XkoHD9KUUq_YWyuA3A@mail.gmail.com>

On (10/29/12 11:32), Nitin Gupta wrote:
> 
> Nothing should be printed (even a debug only message) for
> the ZRAM_ZERO case. This case can be quite common for certain
> kinds of data and would cause a huge log spew.  Also (!handle) case
> is not the same as zero-filled page case, so this message would
> be misleading.
> 
> So, we should either get rid of this warning entirely or only do
> pr_debug("Read before write ....") for (!handle) case and log nothing
> for ZRAM_ZERO case.
> 

I'd rather remove this message. Will resend.



By the way, about use after-free. I'm afraid you fix is not covering 100% of the
cases.

The problem is with this case:
[..]
334 
335         if (unlikely(clen > max_zpage_size)) {
336                 zram_stat_inc(&zram->stats.bad_compress);
337                 src = uncmem;
338                 clen = PAGE_SIZE;
339         }
340 
[..]

where uncmem could be:
-- kmap'ed page
-- kmalloc'ed page

both of which were unmap'ed/kfree'd before. you moved kfree to the end of the function,
while kunmap_atomic(user_mem) is still happening before src = uncmem/memcpy(cmem, src, clen)
pair.


	-ss

  reply	other threads:[~2012-10-29 18:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-27 16:00 [PATCH 1/2] zram: factor-out zram_decompress_page() function Sergey Senozhatsky
2012-10-29 17:14 ` Nitin Gupta
2012-10-29 17:33   ` Sergey Senozhatsky
2012-10-29 18:05   ` [PATCH 1/2] zram: factor-out zram_decompress_page() function (v2) Sergey Senozhatsky
2012-10-29 18:32     ` Nitin Gupta
2012-10-29 18:57       ` Sergey Senozhatsky [this message]
2012-10-29 19:00       ` [PATCH 1/2] zram: factor-out zram_decompress_page() function (v3) Sergey Senozhatsky
2012-10-30 21:04   ` [PATCH 1/2] zram: factor-out zram_decompress_page() function Sergey Senozhatsky
2012-10-31  3:55     ` Nitin Gupta
2012-10-31  7:05       ` zram: use after free Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121029185708.GA2336@swordfish \
    --to=sergey.senozhatsky@gmail.com \
    --cc=fengguang.wu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ngupta@vflare.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.