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
| ||
|
Message-ID: <CALZtONCSUZiNdZ12XJcSZPPOemGXyc27Fy=BKT6ZAFWwBFgu6w@mail.gmail.com> Date: Thu, 14 Aug 2014 15:11:45 -0400 From: Dan Streetman <ddstreet@...e.org> To: David Horner <ds2horner@...il.com> 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 12:23 PM, David Horner <ds2horner@...il.com> wrote: > 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. actually I wasn't thinking of total_bytes changing, i think it's ok to check the total at that specific point in time, for the reasons you point out above. I was thinking about zram->limit_bytes changing, especially if it's possible to disable the limit (i.e. set it to 0), e.g.: assume currently total_bytes == 1G and limit_bytes == 2G, so there is not currently any danger of going over the limit. Then: thread 1 : if (zram->limit_bytes ...this is true thread 2 : zram->limit_bytes = limit; ...where limit == 0 thread 1 : && total_bytes > zram->limit_bytes) { ...this is now also true thread 1 : incorrectly return -ENOMEM failure It's very unlikely, and a single failure isn't a big deal here since the caller must be prepared to handle a failure. And of course the compiler might reorder those checks. And if it's not possible to disable the limit by setting it to 0 (besides a complete reset of the zram device, which wouldn't happen while this function is running), then there's not an issue here (although, I think being able to disable the limit without having to reset the zram device is useful). Also for setting the max_used_bytes, isn't non-atomically setting a u64 value dangerous (on <64 bit systems) when it's not synchronized between threads? That is, unless the entire zram_bvec_write() function or this section is already thread-safe, and i missed it (which i may have :-) > > 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/
Powered by blists - more mailing lists