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 2652EC3DA59 for ; Wed, 17 Jul 2024 02:37:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 889B36B0095; Tue, 16 Jul 2024 22:37:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 839E26B0099; Tue, 16 Jul 2024 22:37:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 701096B009B; Tue, 16 Jul 2024 22:37:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 4D1156B0095 for ; Tue, 16 Jul 2024 22:37:45 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id E93C2A2DE5 for ; Wed, 17 Jul 2024 02:37:44 +0000 (UTC) X-FDA: 82347684048.12.143B84C Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf14.hostedemail.com (Postfix) with ESMTP id 3BD00100004 for ; Wed, 17 Jul 2024 02:37:42 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=KBzqEeWj; spf=pass (imf14.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721183843; 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=114ba7afWbegVKrpGjSSbtb2X9YUkmvheq7EXrkXlHA=; b=6hoMEEY1Qw5Z+WsBdjuaVZXSAtctd867fQxca+6a31k63MtTgxEHKOCKgvLBoXMdzl5VSl fyaQPhz1UPu8Kg6h5Vlt5FZhAX4N84RQ81OAP6wY+RPgwWb0JCu1iLCDFE+gfnsWuW/k1F FepG+yUwAANPiipiO4OtQlnKcM95Q+0= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=KBzqEeWj; spf=pass (imf14.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721183843; a=rsa-sha256; cv=none; b=IcwP1PNl9jjoX3ONRV1E4ZEyh4Wpq1zEZkVZ7ruoIJJilEKV6itAj4azi3F/CqhtSdK5Kh xgNzJofLanQniTPsF6Zi//2DOC+IhLH1+Zph8RaQAX6/wJRgLchT8Mn1dGJ7B2QPQ+q/VZ XTg3V7Bc5RROdh2ieET1RJEAXBjX9IU= Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2cb4e1dca7aso22181a91.0 for ; Tue, 16 Jul 2024 19:37:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1721183861; x=1721788661; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=114ba7afWbegVKrpGjSSbtb2X9YUkmvheq7EXrkXlHA=; b=KBzqEeWjObyotmLCdD1GsOGFwqJ0VWwIqJMb8AaBbiW4IwNmiqVLCJcBfxIRxX7HLR OrdIGD0uvV+nHlmBVOKZQIkkthF5lCdFSTSsLQERxSh98qpj6Axt1fTDZYK17z5JSHII +4S56ACgjyUV/7X79RAoYKNUlkgz/0uYIvpALK1hmEkH+aJbEwfyNHy/DawCt2UM60Ym 9Rn/VZHPF2dWCwdyHkCnglPrrUCMzrSdHWVhI2Z2AL80GNtecw0zzGlM6g6s2HqfXyOl DokS2t+0gIXObwvZiFV3LslwepQ3CwVl9qPeZAt28ym2sfXGTsqQtaTG2svfCDFNf5PX NvYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721183861; x=1721788661; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=114ba7afWbegVKrpGjSSbtb2X9YUkmvheq7EXrkXlHA=; b=i7/nPiHBaIgQi/91G3xaSc1zhMvX8yLXhWDYGtcUhaCsVTRRaePsV1kANe9+/si7WW Yg/vJquGP1cbLnfAUVQ2/sHK5a8LmvadYqjFZnWiV1/VQJeoINfVBkQwbth2awJMjVLj 4nBFsOiW/V70x26TpmSQd+touwaacRX1jNXcLtuHnNhTS6scBS08jUimgHhx/znglU52 q/Z+Oc3Axn015qzWCdkO4DJbCkTC8+QFgVDEG5aPdlJVLJHkFCmHfsNdoxoPLYzDec3k BpekbxWqWtRcPYl8mgtELUOSKqVShJf7NGkE3lrDs3UJ/vpQH1pCpMu2Vcyb4L1lbmD6 vupA== X-Forwarded-Encrypted: i=1; AJvYcCUGMsM5T6U76CSUd8UxwmzFWnlLdBqPljfwlaPBlOqCqRADSEoYlrNidJymxv+zBgEojfDELBEMVrRySuNIhC6giwY= X-Gm-Message-State: AOJu0Yz/cimdmNmfN+HC1UhVIqupGqAZkR9IAy13tc9008YysVcgTsEn /1nnPNDwVTEdgaOc4LNj2C/pg7MqgQ7A6zKbMaI0uxPgSZfQ5vvvxLT9bkS+doM= X-Google-Smtp-Source: AGHT+IFoQoV9mE+30zu4+WVXGHnLt3EsGGEc8xkdYBi/7jR1nk5tkhHYuIpXRK6XalruUy6GeRmSmw== X-Received: by 2002:a05:6a21:501:b0:1c2:88eb:1d33 with SMTP id adf61e73a8af0-1c3fddbed90mr369794637.6.1721183860728; Tue, 16 Jul 2024 19:37:40 -0700 (PDT) Received: from [10.255.168.175] ([139.177.225.248]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fc0bb6ff95sm67292495ad.6.2024.07.16.19.37.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 19:37:40 -0700 (PDT) Message-ID: <1823a31d-eecd-4877-ae3e-a81f95f06501@bytedance.com> Date: Wed, 17 Jul 2024 10:37:35 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: list_lru: Fix NULL pointer dereference in list_lru_add() Content-Language: en-US To: Youling Tang Cc: Kent Overstreet , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Youling Tang References: <20240712032554.444823-1-youling.tang@linux.dev> <56a81429-4e1e-46f9-8844-acb1afd66952@bytedance.com> <68e2ecad-e8ee-4dd5-a49e-8f8507d4e03c@linux.dev> From: Qi Zheng In-Reply-To: <68e2ecad-e8ee-4dd5-a49e-8f8507d4e03c@linux.dev> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 3BD00100004 X-Stat-Signature: g87ud9rr5jbmrc3jagkqiwh7zcm543nx X-Rspam-User: X-HE-Tag: 1721183861-919766 X-HE-Meta: U2FsdGVkX1/q3ZJk4b8psX0jqSJVbURu4zY4oMNt3SJoHdfmsjT21aeZX47qvHFWATIH7XthOgorsdo0+gTbTAZAm2liHDybcLlV4UN7O0QwD4dU8u2CJZmp+8rpA2fVO+DUYjlTaXy9yOu5FKdZL9wjxgYWZUtzb+Egy8ZkBXAJ6Rm8xK28QlsLJMvn02d8u+unmvvyknRfCKgLcTJAPJSlRhMF00AjUSQHiHN77Qdlthj8nrbx+xcyK9NVsGFKbsRYIqTSvonjNK/qowkf5zvmTLc+xXZ+PA3ys+PGaIu/BWPzWJNz44900YKTYFH7m5DQupVDS9dslXamJ5RxsqqePLR6vP3RxREDcORvzRduuok0HG3xtMPeHj+HFJr1cTPe3B41qwEqeQWG4fd9IMMg7J3Tn1mr8OR8Zl/AIgTUimau0PnQCm/Y+aLnIjfwP33ej4niuqVf19kZ/xxEYuuklC6XMa849kPd11NoOjG4E6GltwgVZIyFQcRUx+E75ZgAgRsBTWWFknNl8mN9QzOfmHvUCqMwk0As/l5I8+Q9rrMwBSwZhx+HwGvVhaYUhETNa2vfp6PiNtmXIqtSuoWEb32qCKXF+IOBUZKltPFWyXb3OFrG5ak8tZ2yD9Hcj5d9xNM4GkOVTW+xcJ/efDmQPJxpwH59NmMTItQoZm7jMscJ9luzMgaghx9I24ugDtuLLqBwCJb2ACD+QaXL0pD71e093mtaXpUGqHbyOHghcBa7q93DvtlA1nYLv0teCQb5t9JCP+LG39smtbqUBxWhoS67MOOfajgwBGvTc3g6suKs/OH+9Fe4budvWgAty07NDirc72EajRIYQ7SYbzz00k0zQyLcZg5WeyOP+lVoYArerzzlD4w8DOMXIwMEJ0r6zg2e8X39JOFXV7B8LYYzCsLViNIBTYpIF7oe1gUVPUtDcJ5RxYkVpoKnydjQGUZji8LR1rMuMnFz1cb 0yhXYtoc ME1hEImY9GX2AZaZzVBKNNmjD5bPKsRTGiNiTOyo9eSUeCWQaozy7dwoWAL+QbXUZqg0Jw9+mLS2BojB5/oEYRx3a3z6lW/xURujis7BlLlWfmnZZ8g9Ha7Qa/2jmOvOL2BGNOQBttC8Ykcru6EJnLEryiq0YTxCggqCJiVGLTljukfUuYIp1Fkcwl/LdeX/Cl6qyUJe1Zu47Y40XZ/HFP7gGz1W7RkDgrRLwKRX0uHyi4rhJLQdJGa5KZjSONsV+T+T4VaiBeQcsedOtbU6pCMUGFGmBO94pQ9a4FOg5c7qLEDwyCu6csdaoobx4rG6F++0CAprGs07hYa6XZMHXSp5GteY+bdyZkd+DuhsrygXm1RkHDZvBih/NWjsoFyeEHKlc4+c2h9HX1WPCFzag0eFLpBkH3gGlr0mN4rDCdAvAfGM6yESCzjA+Y7nKEW0KQJmX 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: List-Subscribe: List-Unsubscribe: On 2024/7/17 10:25, Youling Tang wrote: > On 15/07/2024 11:27, Qi Zheng wrote: >> >> >> On 2024/7/12 12:07, Kent Overstreet wrote: >>> On Fri, Jul 12, 2024 at 11:25:54AM GMT, Youling Tang wrote: >>>> From: Youling Tang >>>> >>>> Note that list_lru_from_memcg_idx() may return NULL, so it is necessary >>>> to error handle the return value to avoid triggering NULL pointer >>>> dereference BUG. >>>> >>>> The issue was triggered for discussion [1], >>>> Link [1]: >>>> https://lore.kernel.org/linux-bcachefs/84de6cb1-57bd-42f7-8029-4203820ef0b4@linux.dev/T/#m901bb26cdb1d9d4bacebf0d034f0a5a712cc93a6 >>> >>> I see no explanation for why this is the correct fix, and I doubt it is. >>> What's the real reason for the NULL lru_list_one, and why doesn't this >>> come up on other filesystems? >> >> Agree, IIRC, the list_lru_one will be pre-allocated in the allocation >> path of inode/dentry etc. > Yes, this issue does not fix why lru_list_one is NULL. > > lru_list_one is NULL because the inode is allocated using > kmem_cache_alloc() > instead of kmem_cache_alloc_lru(), and the lru argument passed to > slab_alloc_node() is NULL. See [1] for the actual fix. > > However, the return value of list_lru_from_memcg_idx() may be NULL. When > list_lru_one is NULL, the kernel will panic directly. Do we need to add > error handling when list_lru_one is NULL in list_lru_{add, del}? Nope, we should pre-allocated the list_lru_one before calling list_lru_add(). > > To avoid hiding the actual BUG(previous changes), we might make the > following > changes, Even if you do this, you should still modify all callers to handle this failure. > > diff --git a/mm/list_lru.c b/mm/list_lru.c > index 3fd64736bc45..fa86d3eff90b 100644 > --- a/mm/list_lru.c > +++ b/mm/list_lru.c > @@ -94,6 +94,9 @@ bool list_lru_add(struct list_lru *lru, struct > list_head *item, int nid, >         spin_lock(&nlru->lock); >         if (list_empty(item)) { >                 l = list_lru_from_memcg_idx(lru, nid, > memcg_kmem_id(memcg)); > +               if (WARN_ON_ONCE(!l)) > +                       goto out; > + >                 list_add_tail(item, &l->list); >                 /* Set shrinker bit if the first element was added */ >                 if (!l->nr_items++) > @@ -102,6 +105,7 @@ bool list_lru_add(struct list_lru *lru, struct > list_head *item, int nid, >                 spin_unlock(&nlru->lock); >                 return true; >         } > +out: >         spin_unlock(&nlru->lock); >         return false; >  } > @@ -126,12 +130,16 @@ bool list_lru_del(struct list_lru *lru, struct > list_head *item, int nid, >         spin_lock(&nlru->lock); >         if (!list_empty(item)) { >                 l = list_lru_from_memcg_idx(lru, nid, > memcg_kmem_id(memcg)); > +               if (WARN_ON_ONCE(!l)) > +                       goto out; > + >                 list_del_init(item); >                 l->nr_items--; >                 nlru->nr_items--; >                 spin_unlock(&nlru->lock); >                 return true; >         } > +out: >         spin_unlock(&nlru->lock); >         return false; >  } > > Link: > [1]: > https://lore.kernel.org/all/20240716025816.52156-1-youling.tang@linux.dev/ > > Thanks, > Youling.