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 4A30ECCA47B for ; Mon, 11 Jul 2022 18:36:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D1B7E940015; Mon, 11 Jul 2022 14:36:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CCC4F940010; Mon, 11 Jul 2022 14:36:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB909940015; Mon, 11 Jul 2022 14:36:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id ACC59940010 for ; Mon, 11 Jul 2022 14:36:56 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 765F147B for ; Mon, 11 Jul 2022 18:36:56 +0000 (UTC) X-FDA: 79675675632.18.C4F3524 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf07.hostedemail.com (Postfix) with ESMTP id AF29A4003B for ; Mon, 11 Jul 2022 18:36:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1657564615; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=b3DLjSAmDFAE4pAYUOg4L/VaE2fas/IchGct3GHDmow=; b=GnBaRBPd0e5CzimITR3ImAyVuq9LW6a0d6qycNQG/benntyPeUvRNZGTpbZyCCPscAqxtJ OczM9qrMUhh4We4FR9OaxX5NJnCQNBX+jKE/Sgo8XY91qHAEUYAmaoM2jjwS/ALx6VENU+ +0XaVI01+KF+PGBEp+jNXlX7qroEwcA= Received: from mail-ua1-f70.google.com (mail-ua1-f70.google.com [209.85.222.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-637-D6BIN76yO0aC4_cWdx0zxQ-1; Mon, 11 Jul 2022 14:36:53 -0400 X-MC-Unique: D6BIN76yO0aC4_cWdx0zxQ-1 Received: by mail-ua1-f70.google.com with SMTP id 33-20020ab00424000000b003829f08fd3fso1152263uav.22 for ; Mon, 11 Jul 2022 11:36:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=b3DLjSAmDFAE4pAYUOg4L/VaE2fas/IchGct3GHDmow=; b=tbj8hugg18w2tNH4DZkwlYM8CjjhHRNW/hfNJXpkTsRiA6oLeQ5VevASf7+5n9kJjE Z0UzOVPIOaIm8Angz6ZOPZe0cfnYuNW4Df2E9aHrL6A5OvqMCR1iuKQnq+mFQ23eR59A Sc7ZRv9n5BhjS+plNKI1VaSm2q3V7HTAb0I2PE1kxlHsA6tJM0JheA0ZCGQYDoYmIxf8 wBD4n3dMbX3jH05P+IFQD8YCjVyW7Hm3xIiV8CXRMdcwIQQFytkAdKBmQpT1GIhtwIE9 zaHrFoWo9xrI1Ab/tjTrkerLPnoeLBihKIfufT2uot5Xvx/cVXjgRHEcwRSmS2/y/Q1z p/DA== X-Gm-Message-State: AJIora+9ZBU/oMV9je4KWu9cU0ORMM3pTYcuu5Aza4tLAYMQEGndN7NP ObGCD9/Um55pPvYbiPn/FzP/LOvwjqu3WDJWXUGhYdHXBuDAGRJnmCcS20B5X+EjC50nkJckI9W tsvnpcKmgVvdqeXcU+2fsWc+Oi0Q= X-Received: by 2002:a67:d91d:0:b0:357:37ea:3b02 with SMTP id t29-20020a67d91d000000b0035737ea3b02mr6579946vsj.31.1657564613271; Mon, 11 Jul 2022 11:36:53 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tV42Xw65G2DmR6/SfT6i4pLUhqLcY1/HKPrngTAAyxSs2f25qKLhPiYWs3KsIj1hdT2XUCLxmXKgpRCEwrk/A= X-Received: by 2002:a67:d91d:0:b0:357:37ea:3b02 with SMTP id t29-20020a67d91d000000b0035737ea3b02mr6579937vsj.31.1657564612987; Mon, 11 Jul 2022 11:36:52 -0700 (PDT) MIME-Version: 1.0 References: <20220711075225.15687-1-mlombard@redhat.com> In-Reply-To: From: Maurizio Lombardi Date: Mon, 11 Jul 2022 20:36:42 +0200 Message-ID: Subject: Re: [PATCH] mm: prevent page_frag_alloc() from corrupting the memory To: Alexander Duyck Cc: Andrew Morton , Chen Lin , Jakub Kicinski , LKML , Netdev , linux-mm X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="000000000000f32e5905e38bd51b" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657564616; a=rsa-sha256; cv=none; b=Hce0bERuIEpmo/Y/sthdYrN1gb/1pIinrAHa8r0JqHmVUBjUJkwldrGt8SyHywqaxcm7jR T2aaycb5XyEGiHyPS3NGlsCCm0KqXtdTREea196PpIdP7NUeMufFGNJTDhMOeAJZcfbpn/ I68eCowob6QIqUATn0eILylgCZ+S1Zk= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=GnBaRBPd; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf07.hostedemail.com: domain of mlombard@redhat.com has no SPF policy when checking 170.10.129.124) smtp.mailfrom=mlombard@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1657564616; 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=b3DLjSAmDFAE4pAYUOg4L/VaE2fas/IchGct3GHDmow=; b=AE1uhUXiHe0Eqk13fRTww9CCIRQ2ydX/zu6/xOkrwkGFCBTRfAYpaU8wk+4T90NwLcoWJO gEbl4WFbol+u4pTyKt6BmutQP4ZE/mlVX6QE5MfocxUUTywQZgcreCAsPa6XrktqzBkdY6 WIUf8dEj80OeSiOZZGZ6PvnyDQEdIjs= X-Stat-Signature: oa4f86j5fnddwmp63ndhnsjbpj9x9w1a Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=GnBaRBPd; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf07.hostedemail.com: domain of mlombard@redhat.com has no SPF policy when checking 170.10.129.124) smtp.mailfrom=mlombard@redhat.com X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: AF29A4003B X-Rspam-User: X-HE-Tag: 1657564615-461154 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: --000000000000f32e5905e38bd51b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable po 11.=E2=80=AF7.=E2=80=AF2022 v 20:23 odes=C3=ADlatel Alexander Duyck napsal: > On Mon, Jul 11, 2022 at 9:17 AM Maurizio Lombardi > wrote: > > > > po 11. 7. 2022 v 17:34 odes=C3=ADlatel Alexander Duyck > > napsal: > > > > > > Rather than forcing us to free the page it might be better to move th= e > > > lines getting the size and computing the offset to the top of the "if > > > (unlikely(offset < 0)) {" block. Then instead of freeing the page we > > > could just return NULL and don't have to change the value of any > > > fields in the page_frag_cache. > > > > > > That way a driver performing bad requests can't force us to start > > > allocating and freeing pages like mad by repeatedly flushing the > > > cache. > > > > > > > I understand. On the other hand, if we free the cache page then the > > next time __page_frag_cache_refill() runs it may be successful > > at allocating the order=3D3 cache, the normal page_frag_alloc() behavio= ur > will > > therefore be restored. > > That is a big "maybe". My concern is that it will actually make memory > pressure worse by forcing us to reduce the number of uses for a lower > order page. One bad actor will have us flushing memory like mad so a > guy expecting a small fragment may end up allocating 32K pages because > someone else is trying to allocate them. > > I recommend we do not optimize for a case which this code was not > designed for. Try to optimize for the standard case that most of the > drivers are using. These drivers that are allocating higher order > pages worth of memory should really be using alloc_pages. Using this > to allocate pages over 4K in size is just a waste since they are not > likely to see page reuse which is what this code expects to see. Ok, thanks for the review, I will submit a V2 soon. Maurizio --000000000000f32e5905e38bd51b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


po 11.=E2=80=AF7.=E2=80=AF2022 v 20:23 odes=C3=ADlatel Alex= ander Duyck <alexander.duyc= k@gmail.com> napsal:
On Mon,= Jul 11, 2022 at 9:17 AM Maurizio Lombardi <mlombard@redhat.com> wrote:
>
> po 11. 7. 2022 v 17:34 odes=C3=ADlatel Alexander Duyck
> <ale= xander.duyck@gmail.com> napsal:
> >
> > Rather than forcing us to free the page it might be better to mov= e the
> > lines getting the size and computing the offset to the top of the= "if
> > (unlikely(offset < 0)) {" block. Then instead of freeing = the page we
> > could just return NULL and don't have to change the value of = any
> > fields in the page_frag_cache.
> >
> > That way a driver performing bad requests can't force us to s= tart
> > allocating and freeing pages like mad by repeatedly flushing the<= br> > > cache.
> >
>
> I understand. On the other hand, if we free the cache page then the > next time __page_frag_cache_refill() runs it may be successful
> at allocating the order=3D3 cache, the normal page_frag_alloc() behavi= our will
> therefore be restored.

That is a big "maybe". My concern is that it will actually make m= emory
pressure worse by forcing us to reduce the number of uses for a lower
order page. One bad actor will have us flushing memory like mad so a
guy expecting a small fragment may end up allocating 32K pages because
someone else is trying to allocate them.

I recommend we do not optimize for a case which this code was not
designed for. Try to optimize for the standard case that most of the
drivers are using. These drivers that are allocating higher order
pages worth of memory should really be using alloc_pages. Using this
to allocate pages over 4K in size is just a waste since they are not
likely to see page reuse which is what this code expects to see.

Ok, thanks for the review, = I will submit a V2 soon.

Maurizio=C2=A0
--000000000000f32e5905e38bd51b--