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 06115C47DAF for ; Fri, 19 Jan 2024 15:48:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8E6816B0087; Fri, 19 Jan 2024 10:48:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 897466B0089; Fri, 19 Jan 2024 10:48:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7384B6B008A; Fri, 19 Jan 2024 10:48:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 6382F6B0087 for ; Fri, 19 Jan 2024 10:48:55 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B6D741A03C4 for ; Fri, 19 Jan 2024 15:48:54 +0000 (UTC) X-FDA: 81696493788.08.D5AD8B1 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by imf27.hostedemail.com (Postfix) with ESMTP id 4F0984001B for ; Fri, 19 Jan 2024 15:48:52 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=quicinc.com header.s=qcppdkim1 header.b="nv f669u"; spf=pass (imf27.hostedemail.com: domain of quic_charante@quicinc.com designates 205.220.180.131 as permitted sender) smtp.mailfrom=quic_charante@quicinc.com; dmarc=pass (policy=none) header.from=quicinc.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705679332; 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=J9ig2IJPl4P+QEzFCNblDiqJod4+v+VvpLkQeSXtsGc=; b=BqVHeRWPUgfvOq4AnPw4b1SJQAd/UsiFnBrmkQ+jKTFSbxQdgkIMIP26vTPtqKSlcgfZ4f FL/Ix5EGOTl++qJh9hQkJ0ZgbbqJARxwndslpYP6Q9b2JUpTO8yeKFLyt9CPDFwo3mwf7n tX9jimAKV1rK/Hv+YYgrqqZ9Z7I7P/8= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=quicinc.com header.s=qcppdkim1 header.b="nv f669u"; spf=pass (imf27.hostedemail.com: domain of quic_charante@quicinc.com designates 205.220.180.131 as permitted sender) smtp.mailfrom=quic_charante@quicinc.com; dmarc=pass (policy=none) header.from=quicinc.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705679332; a=rsa-sha256; cv=none; b=B8nKShfSGCNrwDWG9G8sXHObfs5r1QMamWPfkvpSr2hGjIpfVwD9iDhkw//L7S1y9erpBe IpiJapoDYHXgS9CtBXBrs5XvW640XAet+GxWTa/tmbe+2B18fwYFJV1f+lwnWjMyAlQe6q 7POdH9kqN27QWUbT7F0Nku0YCsVAzzU= Received: from pps.filterd (m0279870.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 40JFMamB013798; Fri, 19 Jan 2024 15:48:41 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h= message-id:date:mime-version:subject:to:cc:references:from :in-reply-to:content-type:content-transfer-encoding; s= qcppdkim1; bh=J9ig2IJPl4P+QEzFCNblDiqJod4+v+VvpLkQeSXtsGc=; b=nv f669uktnYhKxrOtL5tcoVJYreCLcVtLMFK7Gqhue0TlFOFgHinaQW6+oagC5bq/X YxejjpXot7W9LnbNV48FnRMqQ+z9iJfIGwAeKxVZbMxM3AAfi4XYHZ/L7UsEXrqP b4WSVp6Y6I6eUSE4KX1RkTIaUmcybRgzltSmcXOiXYX+K8/MZwz9p6AjybPUmPqp OeSpc/11hFlJxoOf/muge1+cfAl7fSbbSTOFeyvwMut8fSi2U8PCeUmGWP1eQFtG KVfcZvO59E7UTo85rLYk6n0tWoxh6TWwDmEzb97ibGfCkuIcIinpAQzmqOh30xxe Y9tFM53BQyAGTW7WKnYw== Received: from nalasppmta03.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3vq403k6vj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 19 Jan 2024 15:48:40 +0000 (GMT) Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA03.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 40JFmdcT004408 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 19 Jan 2024 15:48:39 GMT Received: from [10.216.49.108] (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.40; Fri, 19 Jan 2024 07:48:36 -0800 Message-ID: Date: Fri, 19 Jan 2024 21:18:32 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH] fs: improve dump_mapping() robustness To: Baolin Wang , Al Viro CC: , , , , , , References: <937ab1f87328516821d39be672b6bc18861d9d3e.1705391420.git.baolin.wang@linux.alibaba.com> <20240118013857.GO1674809@ZenIV> Content-Language: en-US From: Charan Teja Kalla In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: QnU7MX2vPrWySRf9jhtVFKZ2LZqmokq7 X-Proofpoint-ORIG-GUID: QnU7MX2vPrWySRf9jhtVFKZ2LZqmokq7 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-01-19_09,2024-01-19_02,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 clxscore=1011 lowpriorityscore=0 priorityscore=1501 mlxscore=0 bulkscore=0 phishscore=0 spamscore=0 adultscore=0 mlxlogscore=741 malwarescore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2311290000 definitions=main-2401190088 X-Rspamd-Queue-Id: 4F0984001B X-Rspam-User: X-Stat-Signature: 5xyqzx1m87oy6f16nef7awz87no1g8fr X-Rspamd-Server: rspam01 X-HE-Tag: 1705679332-811618 X-HE-Meta: U2FsdGVkX19Ttdd/ol5u1QdHbR8gop9/bqXV4r+MIT6F3Dowg0542heK3LqIBanuOVzxypqhz1pmc1Un0NqrYNUb6iVSExUTeP+MtWlhD+7RZR+7qaavgJWhcDQjCyRWrhni18vASZfg63cPQUSJlj1pDQhT75RN3zIuTEmaVgByoJnSiDEtsvGpsFWYFeuzRWqyW+UZTFqog9G62aRBmnQgkLw4RM75BH99PBcXOBsMNnzXZGFg3Dm4ds2v11v5oQm/+TAGb8XIJudK8V6T31yRBP1FRT9XBJOr031eNJ1raNRvN/0dtiUTWHVhRiagG+6ZGBH/C/OzxgQluhhGA5Uw2cwl54eZ/vITRZR1RcrNUv11WHFlT27qFK3zXTu/2OvsLV7LxVxnJ7AuK35V/peueJQ9B0FAfbe+1vbpJk81C4ZxEoI2hitYvH7Ij9pFREgauPkXGcqwi89yNDcU39Kezrmklr5u5Gv4K5VRBHmyOJNbGFT1DrD/aTg2P2daUpC2GnFY47iEyUrYHQcjMKeK9hJ91HOKco0PBydSC79vmgmN3sPrWvFA4g7Rc9Yq4/Bik5VUxxQw++uaCPM09TRbAvFxtxkEISbbh64ZxWFaJ+K2MW8IFuMfhnJnoZ7ChgfXZJ7RacnFffx4G2ZNwakMTLsQal7Dfy3MvaeT69V0b5hi2PCE3dsRbm2CuJSbF1VQkwmi+Dre5vpOgJdm4s84MBo4ep4HKF5ZuT6IsGCMEP0accC6776uYrqAUxfdmmk6YrblsesdR4/ZbYwGvBHmhrM4ZvfmSWjEAViRejnzjhECNLKQjcONg5S7sL9tdEeXnk4MRtoOPhUqCp0SGrpuwnNs74YtuDe/M9/aoMUkXUuLu8ossJpeYV/71D1bgRaWewFSnTsQ1A0Biz7YSOaU5MA5Wnlgq9eZdeTqir2ZNrlt9G+4+/v20nmi3U89H3MT+S1XtuviVpqU4bH VKGbKkMi MkN3Dws5DHVAC4Pg/hFrdgk1/j5+Q6Z8BcYvv9u7I5RCecc8pBl4sU30YMRwej48tJa+ra2Po+/eHRra1nni4GpJrNrdJrtCtc4jx0Nvm4PQ1TUoFlxgTadD7or66WkYZVWtjS10J4m5b+CCG827kiPS/RUx4z4zGv5O+itpSoxofIWIEW7HwLR4+7iBowJimq3rKgeogBvSzS1Nj0lGa0pNE21rYygqS/y9BPKiAQ53VnV6lSugKYepb814r0qrYqU+ojj6+JzRzk+DfuXnCpa6PCLB2cHKj9Woc 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: Hi Matthew/Baolin, On 1/18/2024 8:13 AM, Baolin Wang wrote: > > > On 1/18/2024 9:38 AM, Al Viro wrote: >> On Tue, Jan 16, 2024 at 03:53:35PM +0800, Baolin Wang wrote: >> >>> With checking the 'dentry.parent' and 'dentry.d_name.name' used by >>> dentry_name(), I can see dump_mapping() will output the invalid dentry >>> instead of crashing the system when this issue is reproduced again. >> >>>       dentry_ptr = container_of(dentry_first, struct dentry, >>> d_u.d_alias); >>> -    if (get_kernel_nofault(dentry, dentry_ptr)) { >>> +    if (get_kernel_nofault(dentry, dentry_ptr) || >>> +        !dentry.d_parent || !dentry.d_name.name) { >>>           pr_warn("aops:%ps ino:%lx invalid dentry:%px\n", >>>                   a_ops, ino, dentry_ptr); >>>           return; >> >> That's nowhere near enough.  Your ->d_name.name can bloody well be >> pointing >> to an external name that gets freed right under you.  Legitimately so. >> >> Think what happens if dentry has a long name (longer than would fit into >> the embedded array) and gets renamed name just after you copy it into >> a local variable.  Old name will get freed.  Yes, freeing is RCU-delayed, >> but I don't see anything that would prevent your thread losing CPU >> and not getting it back until after the sucker's been freed. > > Yes, that's possible. And this appears to be a use-after-free issue in > the existing code, which is different from the issue that my patch > addressed. > > So how about adding a rcu_read_lock() before copying the dentry to a > local variable in case the old name is freed? > We too seen the below crash while printing the dentry name. aops:shmem_aops ino:5e029 dentry name:"dev/zero" flags: 0x8000000000080006(referenced|uptodate|swapbacked|zone=2|kasantag=0x0) raw: 8000000000080006 ffffffc033b1bb60 ffffffc033b1bb60 ffffff8862537600 raw: 0000000000000001 0000000000000000 00000003ffffffff ffffff807fe64000 page dumped because: migration failure migrating pfn aef223 failed ret:1 page:000000009e72a120 refcount:3 mapcount:0 mapping:000000003325dda1 index:0x1 pfn:0xaef223 memcg:ffffff807fe64000 Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 Mem abort info: ESR = 0x0000000096000005 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x05: level 1 translation fault Data abort info: ISV = 0, ISS = 0x00000005 CM = 0, WnR = 0 user pgtable: 4k pages, 39-bit VAs, pgdp=000000090c12d000 [0000000000000000] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000 Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP dentry_name+0x1f8/0x3a8 pointer+0x3b0/0x6b8 vsnprintf+0x4a4/0x65c vprintk_store+0x168/0x4a8 vprintk_emit+0x98/0x218 vprintk_default+0x44/0x70 vprintk+0xf0/0x138 _printk+0x54/0x80 dump_mapping+0x17c/0x188 dump_page+0x1d0/0x2e8 offline_pages+0x67c/0x898 Not much comfortable with block layer internals, TMK, the below is what happening in the my case: memoffline dput() (offline_pages) (as part of closing of the shmem file) ------------ -------------------------------------- ....... 1) dentry_unlink_inode() hlist_del_init(&dentry->d_u.d_alias); 2) iput(): a) inode->i_state |= I_FREEING ..... b) evict_inode()->..->shmem_undo_range 1) get the folios with elevated refcount 3) do_migrate_range(): a) Because of the elevated refcount in 2.b.1, the migration of this page will be failed. 2) truncate_inode_folio() -> filemap_remove_folio(): (deletes from the page cache, set page->mapping=NULL, decrement the refcount on folio) b) Call dump_page(): 1) mapping = page_mapping(page); 2) dump_mapping(mapping) a) We unlinked the dentry in 1) thus dentry_ptr from host->i_dentry.first is not a proper one. b) dentry name print with %pd is resulting into the mentioned crash. At least in this case, I think __this patchset in its current form can help us__. Thanks, Charan