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 A0C37D5CC9A for ; Wed, 30 Oct 2024 11:47:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 159196B0082; Wed, 30 Oct 2024 07:47:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 106266B0083; Wed, 30 Oct 2024 07:47:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC2426B0088; Wed, 30 Oct 2024 07:47:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id C5A7A6B0082 for ; Wed, 30 Oct 2024 07:47:35 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 0D2C7140BAA for ; Wed, 30 Oct 2024 11:47:35 +0000 (UTC) X-FDA: 82730093418.29.7483380 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf11.hostedemail.com (Postfix) with ESMTP id A8AA440021 for ; Wed, 30 Oct 2024 11:47:01 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BOFjueHu; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of marc.c.dionne@gmail.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=marc.c.dionne@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730288809; a=rsa-sha256; cv=none; b=ocEA8tMe3HBea5BHNJ6dGaHsj+73Xd6cP2eTVof74cQexIyWWh5akNR5k+msatVa5R4phN Nu9z+5Fk8nAzbVDUOGNjNKWFuQNQre45OvKYeRcaE/15e+yQrJmqOz+w8tlJ6K8iEzE+cE GmscXWKOnCCZRv6a52o0oSYTLYiZpJ8= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BOFjueHu; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of marc.c.dionne@gmail.com designates 209.85.218.53 as permitted sender) smtp.mailfrom=marc.c.dionne@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730288809; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=+iPeoiKgqWsihw7QOhYcfHs+xFsoMwxQZ4v+dLUj0kU=; b=qRlOqdvsJGSvX4dzK8v8MC2MCLm8JIHRJj+dRcPOlEwph6OKVe/NpP9BhSTSqSYP8aYzQm BzMd1g4XFOn4gJyFwRDqI7PvdODTlKHoH7OoIid3kN2OaVkwbRTbdyz1CrN0sftphhkgbJ pX7il/leV+EMEJpyUZpD6WJHPaYmMQg= Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-a9a68480164so919977566b.3 for ; Wed, 30 Oct 2024 04:47:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730288852; x=1730893652; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=+iPeoiKgqWsihw7QOhYcfHs+xFsoMwxQZ4v+dLUj0kU=; b=BOFjueHubg4lK+4zy+SPUMCxY1FXrOEN19z/esnm5OS9j6Myt8LRU6O2DcPvMFA2VD WMm3hhJLFLOgUe31JpdMVSyPg5rQSCdOOB6LlrFrtiJhqNauqu9Jlpvv8Fo4gireinzZ GH46thYoRa04GAQXlmENm43RZYi05TOHPsHrBRy2ngeom0jd0FEEbDQkyuY7d0epbT3f 0bQp86VML0jPiai6XFri6Hz+hRF7/IJizco3CH9b1lzO0YRQa51JCXP/9A2tWrzHSy2X ohPP98zu0dpDvPnAn3qWdagnh8VHy/HNe6jKSM0R2QhupAjeY95g2wexjto0/Kw8PnH5 2GFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730288852; x=1730893652; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+iPeoiKgqWsihw7QOhYcfHs+xFsoMwxQZ4v+dLUj0kU=; b=ONB7QvsTsr52NshO/CGNu5L1Ft0qz3DGeH2m+UvTFDISOi/Gn1SYYDWWWPOsmgdBWc jvm49Qy1Rkxq/vozTqQ0Ry9l09i6c2o4xtdDlQp1OW3wObvKCHC1wTspfiupVh1TPBaL q9lrlo+JucjlMJfE6zkCjup4aM8ySmJgOnMHgPmi+w87vT2b9/YemFpn7vwbQOqE1Iqk MGcJnevwHnqkXxRQOsVnaCMp3HcR9lcoQhq1ZAOyBeN5IeAf0V6Q8uVdLHO579MnZcMA v7nk8AIDLyPp9M5g0aOx7cIAwf/wuG8s3Cox3Dachjl5+N5Kakk0nep0NQPdOKds1iPI 0UMw== X-Gm-Message-State: AOJu0YwRx3IDFaN0SvLy8vsSeUgsWTF8UCDJbOGGBtH7xi48gj0Y3ta1 j89UySjsCEl5yNRC1Ul2lQYq55YLFVesHISivtXX/U99J3DCw0qM28bsG7huuADQOyfG9mziIi6 FlrvSPjVGLOo2Hp/i3G+zAHeqBNKA7hn7 X-Google-Smtp-Source: AGHT+IHOhf0jCKXNjWWUFbLvQ8OX9sG6XrtUY0Z+2vfScEF3jOf24p7/NPgnSEG0lkw4bOS9pecA20ZITnp24rCfHHw= X-Received: by 2002:a17:907:7d8b:b0:a9a:5b4:b19e with SMTP id a640c23a62f3a-a9de5ee15damr1543532666b.32.1730288851169; Wed, 30 Oct 2024 04:47:31 -0700 (PDT) MIME-Version: 1.0 References: <20241029161341.14063-1-marc.c.dionne@gmail.com> <20241029174518.60fa703fc9cb304d6e69e9a0@linux-foundation.org> In-Reply-To: <20241029174518.60fa703fc9cb304d6e69e9a0@linux-foundation.org> From: Marc Dionne Date: Wed, 30 Oct 2024 08:47:19 -0300 Message-ID: Subject: Re: [PATCH v2] tools/mm: Fix slabinfo crash when MAX_SLABS is exceeded To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Vlastimil Babka Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: u14a3pn6wxq5igwkqip5gbhkkp3prow7 X-Rspamd-Queue-Id: A8AA440021 X-Rspamd-Server: rspam02 X-HE-Tag: 1730288821-595425 X-HE-Meta: U2FsdGVkX1+CugyKV/5SgCHPXs8cDn0szUtNCBbn/Hkz0kJyXA6vWFWxgkdkYt7gSnOF/MyR16dw8DBDWviXsgoBmA4XLFEh2jTjZN+O8EiZScBOO0WICjKeDWfTksMN6b6zB3qVFmTahVBUZOQaMb7HiQNbt94AP7T0/c2LRaCulMNjwF/KiEqVtsD0kYlZIiezRTgMWbNimE5MJXON6tLtnkexWxVIfxJVny2q9BgfagUlVXPPSayMjiMSIaswib8Ujsg7DKJ0e5kSiqfgivDRtXWV2SC1O0pPE04bbe7ajxKVVTVt0dmfkuQeH8PutTiob0HFf11Xnh2jJ12McFDgSij9wzySyKRBKYg8Xj5mvBYCLHUJbor7CLJ1DBEPqS+oa+acIIgckyFVHux7cDGGcRHQmt3UrVWBM41btWkMthJ0Ofdcp1nRWpfQHR5hNQvQeN5nB1Gx03KYZtzr2NOxlSAvRd4yh+3kkuZ7cfXn6nbvIepFf4cMsp/TDCyJ54VT40FlKXZU++giI59yJ8dLSTgzSxsjwd1q/BGa91a1cdDWx35FD8C3TKYZ+2uNAWc1twUodhNQhCnbPH6IWkIIdmANQfWBZNkgkKDyugBfa6/utVE9oCAO561zx4/pHYLdE+jqzuGpURgJWyt8UG6yGZf/OkX6pKi2trjVHPpJD/a89blcqjDRHvBICnrGPFLj7LFZhpN5Yjt27xwWXf7WBpotM9YfF5s4I0P5bwRdODYnL5Qgl3nvaEFZHxz+x8JZ4d5p++NXi3d72VT1VujOY1DXtH8AkoIHVyjF4bWuRW70+aiktm59E0r/c+UtT5AV4ijlufZJ7R/ApHw9Ju/WYHl372MDbdTUH8K0kLJHZGxpSh1CbZ2qBQ/kVHugPtwHmo5K3fcDeoV4UM36EdKQZr6CkJ3UkkY7mKmoVBXhcPkHv/AJSUdI1x3n4olLu9RkrLzBbHY+bCXebOR wW32Jx67 hIYheLmEaktpc/cIk4vbv5vBFp+hbJTmPct5SgG6O4QrpEFZ2R9YaUHOFtMgn6Z6OSJeGpbavsi2yV0fFd0ARzsC2haAa0yl8d2Hy4ggo+EgNCmkYuT2qrwv2LTI82oMXhOhkl45EWl82gnhML0W+QMR8IthYmXKnYNZ/6bcEVrZtfDg8Kj/xeg2eeXamiRhj2iEMDrVHY6wpAS6wJ4xW+jFCRI6/gRtxQeBQ9uE+HAPBHv70jo44JtufOytkpi/TvZWzm4DWEn4RAMYrzQZ0A612skwwq6saGlvsnmAWYxJ3NrURLXB4opAveIjyajbHoWea4UkoD7R87NYzVdRB2TGH3+DMo+estEK+SGi9OYxbb7x9KUKsUBQ9WDr4Ml+MpNi9KJgpyjP9A0udfUzt9juvtCf8p73FpCpBY93dpJ9jKCmUZ1AwnDFdPdjvAISixGz81hB9+avDPZEaCgI7Wci7kUUB9MPeLrSzS0l9l/7IfSq+GyxI+OOyt2T4+bBeuQtPsau0bqnMbTJNsguv9Pg0UFgLAvuOjyIG X-Bogosity: Ham, tests=bogofilter, spamicity=0.000027, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Oct 29, 2024 at 9:45=E2=80=AFPM Andrew Morton wrote: > > On Tue, 29 Oct 2024 13:13:41 -0300 Marc Dionne = wrote: > > > From: Marc Dionne > > > > The number of slabs can easily exceed the hard coded MAX_SLABS in the > > slabinfo tool, causing it to overwrite memory and crash. > > > > Increase the value of MAX_SLABS, and check if that has been exceeded fo= r > > each new slab, instead of at the end when it's already too late. Also > > move the check for MAX_ALIASES into the loop body. > > Thanks. > > > --- a/tools/mm/slabinfo.c > > +++ b/tools/mm/slabinfo.c > > @@ -21,7 +21,7 @@ > > #include > > #include > > > > -#define MAX_SLABS 500 > > +#define MAX_SLABS 1000 > > That isn't a very large increase. Fair point, given how quickly it has grown, maybe something like 4k would give more headroom. > > > #define MAX_ALIASES 500 > > #define MAX_NODES 1024 > > > > @@ -1240,6 +1240,8 @@ static void read_slab_dir(void) > > p--; > > alias->ref =3D strdup(p); > > alias++; > > + if (alias - aliasinfo > MAX_ALIASES) > > + fatal("Too many aliases\n"); > > break; > > case DT_DIR: > > if (chdir(de->d_name)) > > @@ -1301,6 +1303,8 @@ static void read_slab_dir(void) > > if (slab->name[0] =3D=3D ':') > > alias_targets++; > > slab++; > > + if (slab - slabinfo > MAX_SLABS) > > + fatal("Too many slabs\n"); > > break; > > This could be improved - if the number of slabs is exactly equal to > MAX_SLABS we'll unnecessarily report an error. Wouldn't this > alteration be better? Actually the issue is rather that we'll accept MAX_SLABS + 1 when we shouldn't, so yes the checks should be >=3D, or could also be just =3D=3D, sorry. > > --- a/tools/mm/slabinfo.c~tools-mm-fix-slabinfo-crash-when-max_slabs-is-e= xceeded-fix > +++ a/tools/mm/slabinfo.c > @@ -1228,6 +1228,8 @@ static void read_slab_dir(void) > continue; > switch (de->d_type) { > case DT_LNK: > + if (alias - aliasinfo >=3D MAX_ALIASES) > + fatal("Too many aliases\n"); > alias->name =3D strdup(de->d_name); > count =3D readlink(de->d_name, buffer, sizeof(buf= fer)-1); > > @@ -1240,10 +1242,10 @@ static void read_slab_dir(void) > p--; > alias->ref =3D strdup(p); > alias++; > - if (alias - aliasinfo > MAX_ALIASES) > - fatal("Too many aliases\n"); > break; > case DT_DIR: > + if (slab - slabinfo >=3D MAX_SLABS) > + fatal("Too many slabs\n"); > if (chdir(de->d_name)) > fatal("Unable to access slab %s\n", slab-= >name); > slab->name =3D strdup(de->d_name); > @@ -1305,8 +1307,6 @@ static void read_slab_dir(void) > if (slab->name[0] =3D=3D ':') > alias_targets++; > slab++; > - if (slab - slabinfo > MAX_SLABS) > - fatal("Too many slabs\n"); > break; > default : > fatal("Unknown file type %lx\n", de->d_type); > _ >