From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EE8EC77B73 for ; Sun, 16 Apr 2023 11:44:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 983676B0071; Sun, 16 Apr 2023 07:44:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 90C226B0074; Sun, 16 Apr 2023 07:44:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 786D36B0075; Sun, 16 Apr 2023 07:44:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 654226B0071 for ; Sun, 16 Apr 2023 07:44:30 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 2A6C680206 for ; Sun, 16 Apr 2023 11:44:30 +0000 (UTC) X-FDA: 80687071500.27.06670C8 Received: from mail-pj1-f51.google.com (mail-pj1-f51.google.com [209.85.216.51]) by imf01.hostedemail.com (Postfix) with ESMTP id EEA6140018 for ; Sun, 16 Apr 2023 11:44:27 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=Kqs3VCxJ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.51 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681645468; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=IdPIXHdAAVocJh8z9AALcLC1nJeSwv0FGrEVmVSo0BQ=; b=XVFyJeYpXfUO07K++KniRGkyUx9lYVm6Uo/Ho5+AheI9tUi7HLQyUfBnW81BfPnOWZiMjh eHBD0kUttP0uDTharvIpImj2Ca+NYuXRAnAwS6J80fbJ/XB8y4tazhX4qgolwZ/XFnlKZY hgEtTkFGyecmO+rKQ6LvY3HvqitVAwY= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=Kqs3VCxJ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.51 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681645468; a=rsa-sha256; cv=none; b=IPQbAQKcmgS50Zma+tglpQV1JP2gRLzMwDiex9U/sfcELWjNH+fk7a7/81qdItTCvsYqvy pYApYFaLOUngSdTKqVKNsA7rbwXf8qZn5245gl+7P2dmYmMePIXkcFMonCDvtm2T15T/cs Te4OPE8tUCFg8Nzfl/YMH+r+E2gNF+4= Received: by mail-pj1-f51.google.com with SMTP id w11so23342651pjh.5 for ; Sun, 16 Apr 2023 04:44:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1681645467; x=1684237467; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=IdPIXHdAAVocJh8z9AALcLC1nJeSwv0FGrEVmVSo0BQ=; b=Kqs3VCxJ9BN0RSTS7r4oFyy65PQjkkuzLPdTmkE5xgDLNSXgcH8SFWSjSWuy7Nra9Q /iF11G+4jKfIC8XNZ4pPD4rZKASD7O4LGjH+Uy+6DnkxAT0gZL7oxdXOmCpBdAXhfKlx RWIrjHE12IATbDsgDRmT6yifOBJBjlayVGSVzBsh2OWYT7X+TYaTfJpuBnnvbI1ayI/C l7lOWhR6R9jyBnEw7Az0q3c89DOMECGETGCw8BCkz5YWlsHrfzvy6gO+GVS8UeoNN1QB ifEi/GbCLy1jWdb7iX9WMoPQE5ko2FQQa5AQrc32BdTo2otW9GRGkqO9Cif3mR4S4dtn 1wig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681645467; x=1684237467; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=IdPIXHdAAVocJh8z9AALcLC1nJeSwv0FGrEVmVSo0BQ=; b=OU1Y0SnMHkdpsaItlYP0XKr24xaGridk2X6HvYOkRsR0pYicS+GBjBxngU2vl8vJJ/ mwTpBZZu2vK5fsS+lqKinDWN2Phf4KPzVlDBn4+3iFaL80Do167R32JU2MOMHAryi9Hn 0M0pORC6YdFpFbIwDpaNxp/he5UfpG8hyAu9NnUkLbl2ljvqMcf2dOLMqcLxuA571Z8Q IckYKFqglr8IGzcmHfQaC+m0P0Cw+0o7FyF4CQuWlx77mvt4q8vyHpaSULyl5STaGlpq kcCtz/rIW/92DxdiFB2BjZmR/twFTXznPsZ4LWUzMnjxuhgrp4/6YP/OnZccBHYmTrVC 7ymw== X-Gm-Message-State: AAQBX9egUkeKagUdeK1burvUFqgPSKpTey9FFfCQyRwE/RZovd3zlHEX B56r3E4A461NiSMZUCLxIWQ= X-Google-Smtp-Source: AKy350YNDZ/VSrvcDv9xsz8toTVxILT3xYH3kza7r1DYS+8JyvS1jEiwlGwyHiccbjN5j5rct4uijA== X-Received: by 2002:a17:90b:3142:b0:247:1418:ad28 with SMTP id ip2-20020a17090b314200b002471418ad28mr12252878pjb.22.1681645466537; Sun, 16 Apr 2023 04:44:26 -0700 (PDT) Received: from hyeyoo ([211.108.101.96]) by smtp.gmail.com with ESMTPSA id s2-20020a17090aba0200b00240015b837fsm11961245pjr.2.2023.04.16.04.44.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 16 Apr 2023 04:44:25 -0700 (PDT) Date: Sun, 16 Apr 2023 20:44:14 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: david.keisarschm@mail.huji.ac.il Cc: linux-kernel@vger.kernel.org, Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Roman Gushchin , Jason@zx2c4.com, linux-mm@kvack.org, ilay.bahat1@gmail.com, aksecurity@gmail.com Subject: Re: [PATCH v5 1/3] Replace invocation of weak PRNG Message-ID: References: <20230415173633.5422-1-david.keisarschm@mail.huji.ac.il> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230415173633.5422-1-david.keisarschm@mail.huji.ac.il> X-Rspamd-Queue-Id: EEA6140018 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: z9btzp5yomqtowft8zq3am1dwu1s6taf X-HE-Tag: 1681645467-8665 X-HE-Meta: U2FsdGVkX1+Iblu9rlkvrrxAhOZCGDfMEOaWMOjakTD19nhox/+GZycTi/0G4Td2KWCM/j0EnoO9h9HboC3Ngqk7jF1v2YTQo9/3LxbmN57SkcGl6Iz3U1Eq4Knb11evK6a4OqkrVpGr6nPeM2c1c7AKIrnIxAQRD21sFZfi5haS0vbn6sZxjClq7O+5ewUcrtQ70yCA2UsXcH7+c0q1FEHt/UL5JBlCS5eoiBPms0a0OI4WbDKI/YV4Gjd6T4nr2HAFWfY4Ib19/n3NHYMjSKKHQN3uwI011vsWLdcaGHbEwdzUUKWSHtSGzIEiqTN0/TLpY1yite0j/9R4B8j3Red3G4tV442RWoEx703fcdYNjmSx4RlK+aI0Dmib9IGO7lDcZ0ozJZsUs1Bu7AHS36cQ+WjCVaGxx+u7IJMWdyMzFTv94mStFIJSdtLm3UtRrZkreZUGRbX62Bu3ptb1MKbdwjln6lBHi41FKC1blgArZn0JuuA0iJrtsm0dT/WP4hLd+67kcsTOWrCUMwAeQTuDXyTRIn/5krLegpy1un3MzDTBz3r3mPKFFUg5oP8+gxZBeUUKvce7XMDnPGVOnRePTu8gQmAOCk7wYLR00tCkio455822iSt3oQgL2iXBqL/5a8P+Xqz6OrwwQr4PQY7PQlAVmzrVge744zQemtf7PzlQKTKvjQchvrVyxJt4QL5nIDmms+P63eDKKj9e/kIMFscUuEhjW5vITer0r/yh0919UD1Sl5PVfiq9p75eEDNABUAh5xMnBdvUtJgPNSAi6wx8/ESJR6TsupsluGuVJlMaiZmkKNRkybiFvUvS8tPzl4jo5UgzrFmCOvH5iVmgbUZgGeKbGmjHIjbkSnBz+ypidns1Fr2gJzE/BAwu8mrlpYBnWvPfz/QBqEcjY2TjUUPFde4ZruXuN3MJi/q8tn0VlZ9H1l7diu7cfkpIEAKa+FFFOwcA/iqWo4w 5i+Hn7Wr +ZCL9AJUZTOTKwAZrySGpGzNwLEqEnK6GXInwaWr6GJ+A8jNGli0iPonoIaHUzBnj82GiJgZiW9FRWVeT/y/e5anhq0RV/qu6K02M7tVs5Kj0BLEj7liPrwZjPKd0kw8js1f88LQkO4b/tBI/olWGxlKw2ICnwSENOKDjWYZpmpcxqQsgRc/Gb7/TcEaBjeFjfZN0SXmqMkAUSPxbNa+OjXCVYfmzGzlystFAGzluZtsx3+5ngnwZvHHJ9KTTT8/8Fge1Pim8+kQer/Jn/ZelJAGBETioOSAeeVYikqB/T0UdR5DPNDp+PkX3/XSZjQb2tNkgjGRp+mPg6kqhkWHTt6TqyaTPPAo1RmpduxxCVtAe7wpDkkh9B3IQ6V5ac++gQ4PIJbnMBV6UEXVHiCjke1/4ecVBJx6e6gGPhIwPphm+3PGXeVwRniEnmLibV/4sM3vMKtI/9qxOjgc7QM8EFTY3bxjQBqWSZ6pBDRadgG0H4mb2VCnDoeYdAiUsKc33G09DPdZrLcREOx1NJpRIuDEUKNAGHCkOoKgVR/CG/aBDYsQ= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Apr 15, 2023 at 08:36:32PM +0300, david.keisarschm@mail.huji.ac.il wrote: > From: David Keisar Schmidt > > The Slab allocator randomization uses the prandom_u32 > PRNG. That was added to prevent attackers to obtain information on the heap > state, by randomizing the freelists state. > > However, this PRNG turned out to be weak, as noted in commit c51f8f88d705 > To fix it, we have changed the invocation of prandom_u32_state to get_random_u32 > to ensure the PRNG is strong. Since a modulo operation is applied right after that, > we used get_random_u32_below, to achieve uniformity. > > In addition, we changed the freelist_init_state union to struct, > since the rnd_state inside which is used to store the state of prandom_u32, > is not needed anymore, since get_random_u32 maintains its own state. makes sense to me, but some nits: I think the subject should start with "mm/slab:" for this patch and "mm/slab_common:" for the next patch. > > Signed-off-by: David Keisar Schmidt > --- > This fifth series changes only the arch/x86/mm/kaslr patch. > > Changes since v3: > > * edited commit message. > > Changes since v2: > > * replaced instances of get_random_u32 with get_random_u32_below > in mm/slab.c. > > mm/slab.c | 25 ++++++++++--------------- > 1 file changed, 10 insertions(+), 15 deletions(-) > > diff --git a/mm/slab.c b/mm/slab.c > index edbe722fb..7c8bb4a8f 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -2360,20 +2360,17 @@ static void cache_init_objs_debug(struct kmem_cache *cachep, struct slab *slab) > > #ifdef CONFIG_SLAB_FREELIST_RANDOM > /* Hold information during a freelist initialization */ > -union freelist_init_state { > - struct { > - unsigned int pos; > - unsigned int *list; > - unsigned int count; > - }; > - struct rnd_state rnd_state; > +struct freelist_init_state { > + unsigned int pos; > + unsigned int *list; > + unsigned int count; > }; > > /* > * Initialize the state based on the randomization method available. > * return true if the pre-computed list is available, false otherwise. > */ > -static bool freelist_state_initialize(union freelist_init_state *state, > +static bool freelist_state_initialize(struct freelist_init_state *state, > struct kmem_cache *cachep, > unsigned int count) > { > @@ -2381,23 +2378,22 @@ static bool freelist_state_initialize(union freelist_init_state *state, > unsigned int rand; > > /* Use best entropy available to define a random shift */ > - rand = get_random_u32(); > + rand = get_random_u32_below(count); > > /* Use a random state if the pre-computed list is not available */ This comment should be removed too. > if (!cachep->random_seq) { > - prandom_seed_state(&state->rnd_state, rand); > ret = false; > } else { > state->list = cachep->random_seq; > state->count = count; > - state->pos = rand % count; > + state->pos = rand; this could be: state->pos = get_random_u32_below(count); without defining the variable rand. > ret = true; > } > return ret; > } > > /* Get the next entry on the list and randomize it using a random shift */ > -static freelist_idx_t next_random_slot(union freelist_init_state *state) > +static freelist_idx_t next_random_slot(struct freelist_init_state *state) > { > if (state->pos >= state->count) > state->pos = 0; > @@ -2418,7 +2414,7 @@ static void swap_free_obj(struct slab *slab, unsigned int a, unsigned int b) > static bool shuffle_freelist(struct kmem_cache *cachep, struct slab *slab) > { > unsigned int objfreelist = 0, i, rand, count = cachep->num; > - union freelist_init_state state; > + struct freelist_init_state state; > bool precomputed; > > if (count < 2) > @@ -2447,8 +2443,7 @@ static bool shuffle_freelist(struct kmem_cache *cachep, struct slab *slab) > > /* Fisher-Yates shuffle */ > for (i = count - 1; i > 0; i--) { > - rand = prandom_u32_state(&state.rnd_state); > - rand %= (i + 1); > + rand = get_random_u32_below(i+1); per the coding standard this should be rand = get_random_u32_below(i +1); > swap_free_obj(slab, i, rand); > } > } else { > -- > 2.37.3 >