linux-kernel - Re: [PATCH 3/3] zram: add mem_used

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:	Thu, 14 Aug 2014 12:23:23 -0400
From:	David Horner <ds2horner@...il.com>
To:	Dan Streetman <ddstreet@...e.org>
Cc:	Minchan Kim <minchan@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux-MM <linux-mm@...ck.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Jerome Marchand <jmarchan@...hat.com>, juno.choi@....com,
	seungho1.park@....com, Luigi Semenzato <semenzato@...gle.com>,
	Nitin Gupta <ngupta@...are.org>,
	Seth Jennings <sjennings@...iantweb.net>
Subject: Re: [PATCH 3/3] zram: add mem_used_max via sysfs

On Thu, Aug 14, 2014 at 11:32 AM, David Horner <ds2horner@...il.com> wrote:
> On Thu, Aug 14, 2014 at 11:09 AM, Dan Streetman <ddstreet@...e.org> wrote:
>> On Wed, Aug 13, 2014 at 9:12 PM, Minchan Kim <minchan@...nel.org> wrote:
>>> -       if (zram->limit_bytes &&
>>> -               zs_get_total_size_bytes(meta->mem_pool) > zram->limit_bytes) {
>>> +       total_bytes = zs_get_total_size_bytes(meta->mem_pool);
>>> +       if (zram->limit_bytes && total_bytes > zram->limit_bytes) {
>>
>> do you need to take the init_lock to read limit_bytes here?  It could
>> be getting changed between these checks...
>
> There is no real danger in freeing with an error.
> It is more timing than a race.
I probably should explain my reasoning.

any changes between getting the total value and the limit test are not
problematic (From race perspective).

1) If the actual total increases and the value returned under rates it, then
a) if this.total exceeds the limit - no problem it is rolled back as
it would if the actual total were used.
b) if this.total <= limit OK - as other process will be dinged (it
will see its own allocation)

2)  If the actual total decreases and the value returned overrates
rates it, then
a) if this.value <= limit then allocation great (actual has even more room)
b) if this.value > max it will be rolled back (as the other might be
as well) and process can compete again.

Is there a denial of service possible if 2.b repeats indefinitely.
Yes, but how to set it up reliably? And it is no different than a
single user exhausting the limit before any other users.
Yes, it is potentially a live false limit exhaustion, with one process
requesting an amount exceeding the limit but able to be allocated.
 But this is no worse than the rollback load we already have at the limit.

It would be better to check before the zs_malloc if the concern is
avoiding heavy processing in that function (as an optimization),  as
well as here.after allocation

But I see no real race or danger doing this unlocked.
--
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/