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 C06ACCF9C71 for ; Tue, 24 Sep 2024 23:19:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D7B856B0098; Tue, 24 Sep 2024 19:19:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D2CB86B0099; Tue, 24 Sep 2024 19:19:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BD92D6B009B; Tue, 24 Sep 2024 19:19:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id A1F2A6B0098 for ; Tue, 24 Sep 2024 19:19:45 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 56420ABE05 for ; Tue, 24 Sep 2024 23:19:45 +0000 (UTC) X-FDA: 82601201130.08.547D020 Received: from mail-lf1-f45.google.com (mail-lf1-f45.google.com [209.85.167.45]) by imf23.hostedemail.com (Postfix) with ESMTP id 708A4140006 for ; Tue, 24 Sep 2024 23:19:43 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="HD5/MZ3G"; spf=pass (imf23.hostedemail.com: domain of smfrench@gmail.com designates 209.85.167.45 as permitted sender) smtp.mailfrom=smfrench@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727219948; 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=zqgkO9umAcljezxbBhTinw4cVNyUsl7JtTdhMvNKcDM=; b=HiOVEhrM4S3kP1yNMtLEWzTmBaJdF1iI6bl4ogyO5UMRxv+SEJcT1IMsVPsgtSFBlJLAjH M2lCrBEKznWwURwBOHPcsm4YrsLJphRjnqJ8qCzwyzhs7RJR1KcORc4yvMZkbn3tYqxpVK SkkyjZ+lfYSXHRg9Xl09KCr4aLw3a00= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="HD5/MZ3G"; spf=pass (imf23.hostedemail.com: domain of smfrench@gmail.com designates 209.85.167.45 as permitted sender) smtp.mailfrom=smfrench@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727219948; a=rsa-sha256; cv=none; b=J9SvEt2/Frqb3wrpp063jO91gQdOUOrYOkLAotuQ7NBOax/k+RrLWM9vUQhp6YQDTirGNQ oOB/KUoSKgeP9VKSLEtBVpjtgSS2Lgv4JsNt8x0pDx6ZMriwdkAEpIvbkWXnp1KxCgWlt1 3rqOBB01UPcOubSoezY/V+2iG/lVtLc= Received: by mail-lf1-f45.google.com with SMTP id 2adb3069b0e04-536584f6c84so7523560e87.0 for ; Tue, 24 Sep 2024 16:19:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727219981; x=1727824781; 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=zqgkO9umAcljezxbBhTinw4cVNyUsl7JtTdhMvNKcDM=; b=HD5/MZ3GFrteUW9WCFDSsM3FPn1nENSCoiZyjKig7JV8dou580KPqE8MlVnTcT+PDb 4+6dACNGHEctGbGt2oIChBv9dC2YDLM7GiJ56IBByYfjlwZMOwmhYnMwe8iz1G2UKnlp V8x6FxWBGXz7AxCNuLbm6ZR1DVP1cQCOVB7vYgChDdPBW3N4ecPiS+rvQWazlByAksqY AJPgaqqmM0Z1VvuPr93K8s3hjHTOGMaPH7NVJKsHRU1DvzvPc5iYlkDYZoYZwmifYJjK uMJEjzXw0/26DHssazfuVA/RWg6TjrHpY+ZJmRDlZXcpL7G53BSsty+swWSEqVLkMK19 xUkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727219981; x=1727824781; 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=zqgkO9umAcljezxbBhTinw4cVNyUsl7JtTdhMvNKcDM=; b=VMdLdvPIQtrywtZztEjlyGSufherN1OBMeQfDWu9BgbQCzB5p/vIWs7YEBR+3Aa9z0 m8M61k7obZRAEWnItcKCOIU1qKKx9cafbcdDhrWiYw9Kzl/J1qgJJMTRI4Rx5s164R1C 46+1lnewcC37FVlSC2fwQyLjD/rYEObgGABq0tfOmQFpkX3noQW0z9ZEWGaPf7ysy+WJ HiWkf3x9GnXDt3AhjUifN07ZT4RzzEabkvlflt3/rz69sZNuJC26xwmibzARtbjgXtY+ xBzvpUFmdX+cHu1MN6oQlO6yNI7Sag8QWbVswYTgesTh54b1gRu09x9iLr7nl9Tqy66w lxZA== X-Forwarded-Encrypted: i=1; AJvYcCXRL1XKP2gOZlHD+Izr2KYVAUc830dqAAAwIPlsAeXVZR8FHE38eKIbRwBTKljELze1ulviM/cntw==@kvack.org X-Gm-Message-State: AOJu0YxjE4BWErF/ALogvuRcUSJYjxHGMd7R3TNrr64qBrN58+1hPFZI COK5O5lWNmz0KnEN00cfcDYb/4dZW5RK8TTtkraQM+ekZH91vMSlILF6CU+ebzJpG+vVJTuDQYt fFosH2UX+1uMSeYkl6zte43GAQuU= X-Google-Smtp-Source: AGHT+IEs6SrkoESjM16ll3jZm1hG9iEgirZxprDayiTWCubQB6bBwgWo565ZxYCvymE4dc3N9ZbWjILkkC0/lhIUGyI= X-Received: by 2002:a05:6512:114b:b0:531:4c6d:b8ef with SMTP id 2adb3069b0e04-5387048b8e9mr332583e87.6.1727219981129; Tue, 24 Sep 2024 16:19:41 -0700 (PDT) MIME-Version: 1.0 References: <202409131438.3f225fbf-oliver.sang@intel.com> <1191933.1727214450@warthog.procyon.org.uk> In-Reply-To: <1191933.1727214450@warthog.procyon.org.uk> From: Steve French Date: Tue, 24 Sep 2024 18:19:29 -0500 Message-ID: Subject: Re: [linux-next:master] [netfs] a05b682d49: BUG:KASAN:slab-use-after-free_in_copy_from_iter To: David Howells Cc: kernel test robot , oe-lkp@lists.linux.dev, lkp@intel.com, Linux Memory Management List , Christian Brauner , Jeff Layton , netfs@lists.linux.dev, linux-fsdevel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: ub4qp14ynjhubpbq4fmtfnngzpu31zae X-Rspamd-Queue-Id: 708A4140006 X-Rspamd-Server: rspam11 X-HE-Tag: 1727219983-202174 X-HE-Meta: U2FsdGVkX182EBbrjDtD6TqckFRFybzmxpqAEGm5/0cmAUN4MsYVWst2aK+qEXrzU3lK7ILkKYArhv6OPUDgb4sBJcyQwCWSWgQtkOS145HtQlc0DVIpijo4t+bD07ZpIzBIowEvJvZYgb91ZEGxyejoTozzeBAM9fepNcnS3s1LH/Rjpzj9Gho1ZBol4MAsZLAsVq3/jPq2oTdybR7SRn7XCYdpnfH0ZXD6v6VVlalMoXvyZicikeu7IEkBDItCtCC582a5mMZh8EKV1F6PixEq0BXD368ZdxBB6wlgRQeTPBlKcyLwU646J7vXx5U0SsQ+/pbGWRCumCdO3bFBTd5lLn44VY/aWYNBj+Yk2mMNOwI1Ojy5LkO3dzb+zYRZ18ChwXgeIlHO4gW/iTRQHPSKO/CKthAbF6RVE+7covfb1gA6KETjuzxJfHmYWackApF/81tKzVpnzLa/mhBigpKdMb+EVa4e3mgJ5YhJBNZS6JLQFNXJuws5w+qyT6zR91Q02rCKisesNL6bmjtDai/UsV1VPF/nw9cmv8VZocella7xlhWARLoO4WPd92iPHLFbMqM6lPXQkFd4yWwbV9qsc0HMPg80nCQQRFo0sKDY31reQz6CpNXYcg+MGUs20sbGIGC+pUAXgMoGkJ/WKTBS02TjwCfjbD8RflV0cOfnztsnGBxp1sPuy9MquhlsLacrsHWILOYfwKeJNWPrhECSFH1f/woh/drKf075b4s3WhbXyDXvF4zT+krqXaEVSQbi3EZrJL6hWIXiL6kTq2FG5HNN7DjmFhnxjoQgza2Hwc49Ku5dc7hd7I4ZNSnnPk2mn2LbY+WGdS0VirWooxqmXM1hxwholeAIStG7QuaCqq2HK79V2xIKMMpjS4JVivRuczR90LeiqdtoH9UlQkmcLZBlHVaYhnksud3GpJtV7v9gai68rrGonuhFsIEL/bT1euLfkXbghiAwJEj R9BjJw4/ Ru/NSJveTBQd3UFbx+IUkz7mksqVORam98NoVbYTrQPczixILPMcIDBKb/gt91NdK9c+Upg80V58uQRUzmy4C/8PGqBbWFMl50R8lotP9wrQ1YKUgylUFqW1L88HduI64QzLfQAKH3m+pqN7RdsEHRFYX2zfaLSJfmwz+bgHeC7U4gDl24BsQ9yDSI1gCIa45IBEAd/EvHRfPoTav7Hps279HcXiwlCrEmk5/wmshsq/rSgAkEUfDsr3H+94tGQ7pTmwSMvOUKfWEBAT/ZO5ZoBg6ULVTBaxmrAqTiK3VuZy4uqIw0wQhHUn43nufxW/wzauqOEV43GYS0ZuTFbGeHLG2yrpN+txZR+0S0zBy5r2b9izuxAYHP4sVKKxnErl7jAlhzp/089+vYotOAvZU9bOwe572hw47zd7lMqSjpANYymbm0LKb3rU24GXQkTeGbQ+eFzIjgrQR5z5uAMu6ihY1/i+NxWdzSFMsda21HS/x+Y3qVwcNd0REnlCV7trOmxQkglikJ/nCJT1dUd6DPhOT+zNkJTFzsAbAP9TyMpohG3QPe4Ebq+lDt+HmW0nkZIztoMGP82bCkHjBDr8fGqMASsDqtR6Q0EG3KwrUdEjVc65nnVXlL6P078OsmwpK4FbDWgWkFEaAZoWcS5un0exFWFkV/t7zeyaUrhu3774F2tU1FyLxmFzlAoggfBDnmY2FDQGc3lojyqhAYquzcq6aiKjdB24bf0zAEx+QLISLD5Cw2p0owXsnKjwB1RJl5/bgWCDXcukSjrgt4aYTGfS/0g== 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: Yes - I can confirm that this fixes the recent netfs regression in generic/= 075 http://smb311-linux-testing.southcentralus.cloudapp.azure.com/#/builders/3/= builds/239 You can add: Tested-by: Steve French On Tue, Sep 24, 2024 at 4:47=E2=80=AFPM David Howells = wrote: > > Does the attached fix the problem? > > David > --- > netfs: Fix write oops in generic/346 (9p) and maybe generic/074 (cifs) > > In netfslib, a buffered writeback operation has a 'write queue' of folios > that are being written, held in a linear sequence of folio_queue structs. > The 'issuer' adds new folio_queues on the leading edge of the queue and > populates each one progressively; the 'collector' pops them off the > trailing edge and discards them and the folios they point to as they are > consumed. > > The queue is required to always retain at least one folio_queue structure= . > This allows the queue to be accessed without locking and with just a bit = of > barriering. > > When a new subrequest is prepared, its ->io_iter iterator is pointed at t= he > current end of the write queue and then the iterator is extended as more > data is added to the queue until the subrequest is committed. > > Now, the problem is that the folio_queue at the leading edge of the write > queue when a subrequest is prepared might have been entirely consumed - b= ut > not yet removed from the queue as it is the only remaining one and is > preventing the queue from collapsing. > > So, what happens is that subreq->io_iter is pointed at the spent > folio_queue, then a new folio_queue is added, and, at that point, the > collector is at entirely at liberty to immediately delete the spent > folio_queue. > > This leaves the subreq->io_iter pointing at a freed object. If the syste= m > is lucky, iterate_folioq() sees ->io_iter, sees the as-yet uncorrupted > freed object and advances to the next folio_queue in the queue. > > In the case seen, however, the freed object gets recycled and put back on= to > the queue at the tail and filled to the end. This confuses > iterate_folioq() and it tries to step ->next, which may be NULL - resulti= ng > in an oops. > > Fix this by the following means: > > (1) When preparing a write subrequest, make sure there's a folio_queue > struct with space in it at the leading edge of the queue. A functio= n > to make space is split out of the function to append a folio so that > it can be called for this purpose. > > (2) If the request struct iterator is pointing to a completely spent > folio_queue when we make space, then advance the iterator to the new= ly > allocated folio_queue. The subrequest's iterator will then be set > from this. > > Whilst we're at it, also split out the function to allocate a folio_queue= , > initialise it and do the accounting. > > The oops could be triggered using the generic/346 xfstest with a filesyst= em > on9P over TCP with cache=3Dloose. The oops looked something like: > > BUG: kernel NULL pointer dereference, address: 0000000000000008 > #PF: supervisor read access in kernel mode > #PF: error_code(0x0000) - not-present page > ... > RIP: 0010:_copy_from_iter+0x2db/0x530 > ... > Call Trace: > > ... > p9pdu_vwritef+0x3d8/0x5d0 > p9_client_prepare_req+0xa8/0x140 > p9_client_rpc+0x81/0x280 > p9_client_write+0xcf/0x1c0 > v9fs_issue_write+0x87/0xc0 > netfs_advance_write+0xa0/0xb0 > netfs_write_folio.isra.0+0x42d/0x500 > netfs_writepages+0x15a/0x1f0 > do_writepages+0xd1/0x220 > filemap_fdatawrite_wbc+0x5c/0x80 > v9fs_mmap_vm_close+0x7d/0xb0 > remove_vma+0x35/0x70 > vms_complete_munmap_vmas+0x11a/0x170 > do_vmi_align_munmap+0x17d/0x1c0 > do_vmi_munmap+0x13e/0x150 > __vm_munmap+0x92/0xd0 > __x64_sys_munmap+0x17/0x20 > do_syscall_64+0x80/0xe0 > entry_SYSCALL_64_after_hwframe+0x71/0x79 > > This may also fix a similar-looking issue with cifs and generic/074. > > | Reported-by: kernel test robot > | Closes: https://lore.kernel.org/oe-lkp/202409180928.f20b5a08-oliver.s= ang@intel.com > > Signed-off-by: David Howells > cc: Eric Van Hensbergen > cc: Latchesar Ionkov > cc: Dominique Martinet > cc: Christian Schoenebeck > cc: Steve French > cc: Paulo Alcantara > cc: Jeff Layton > cc: v9fs@lists.linux.dev > cc: linux-cifs@vger.kernel.org > cc: netfs@lists.linux.dev > cc: linux-fsdevel@vger.kernel.org > --- > fs/netfs/internal.h | 2 + > fs/netfs/misc.c | 72 ++++++++++++++++++++++++++++++++++--------= ------- > fs/netfs/objects.c | 12 ++++++++ > fs/netfs/write_issue.c | 12 +++++++- > 4 files changed, 76 insertions(+), 22 deletions(-) > > diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h > index c7f23dd3556a..79c0ad89affb 100644 > --- a/fs/netfs/internal.h > +++ b/fs/netfs/internal.h > @@ -58,6 +58,7 @@ static inline void netfs_proc_del_rreq(struct netfs_io_= request *rreq) {} > /* > * misc.c > */ > +struct folio_queue *netfs_buffer_make_space(struct netfs_io_request *rre= q); > int netfs_buffer_append_folio(struct netfs_io_request *rreq, struct foli= o *folio, > bool needs_put); > struct folio_queue *netfs_delete_buffer_head(struct netfs_io_request *wr= eq); > @@ -76,6 +77,7 @@ void netfs_clear_subrequests(struct netfs_io_request *r= req, bool was_async); > void netfs_put_request(struct netfs_io_request *rreq, bool was_async, > enum netfs_rreq_ref_trace what); > struct netfs_io_subrequest *netfs_alloc_subrequest(struct netfs_io_reque= st *rreq); > +struct folio_queue *netfs_folioq_alloc(struct netfs_io_request *rreq, gf= p_t gfp); > > static inline void netfs_see_request(struct netfs_io_request *rreq, > enum netfs_rreq_ref_trace what) > diff --git a/fs/netfs/misc.c b/fs/netfs/misc.c > index 0ad0982ce0e2..a743e8963247 100644 > --- a/fs/netfs/misc.c > +++ b/fs/netfs/misc.c > @@ -9,34 +9,64 @@ > #include "internal.h" > > /* > - * Append a folio to the rolling queue. > + * Make sure there's space in the rolling queue. > */ > -int netfs_buffer_append_folio(struct netfs_io_request *rreq, struct foli= o *folio, > - bool needs_put) > +struct folio_queue *netfs_buffer_make_space(struct netfs_io_request *rre= q) > { > - struct folio_queue *tail =3D rreq->buffer_tail; > - unsigned int slot, order =3D folio_order(folio); > + struct folio_queue *tail =3D rreq->buffer_tail, *prev; > + unsigned int prev_nr_slots =3D 0; > > if (WARN_ON_ONCE(!rreq->buffer && tail) || > WARN_ON_ONCE(rreq->buffer && !tail)) > - return -EIO; > - > - if (!tail || folioq_full(tail)) { > - tail =3D kmalloc(sizeof(*tail), GFP_NOFS); > - if (!tail) > - return -ENOMEM; > - netfs_stat(&netfs_n_folioq); > - folioq_init(tail); > - tail->prev =3D rreq->buffer_tail; > - if (tail->prev) > - tail->prev->next =3D tail; > - rreq->buffer_tail =3D tail; > - if (!rreq->buffer) { > - rreq->buffer =3D tail; > - iov_iter_folio_queue(&rreq->io_iter, ITER_SOURCE,= tail, 0, 0, 0); > + return ERR_PTR(-EIO); > + > + prev =3D tail; > + if (prev) { > + if (!folioq_full(tail)) > + return tail; > + prev_nr_slots =3D folioq_nr_slots(tail); > + } > + > + tail =3D netfs_folioq_alloc(rreq, GFP_NOFS); > + if (!tail) > + return ERR_PTR(-ENOMEM); > + tail->prev =3D prev; > + if (prev) > + /* [!] NOTE: After we set prev->next, the consumer is ent= irely > + * at liberty to delete prev. > + */ > + WRITE_ONCE(prev->next, tail); > + > + rreq->buffer_tail =3D tail; > + if (!rreq->buffer) { > + rreq->buffer =3D tail; > + iov_iter_folio_queue(&rreq->io_iter, ITER_SOURCE, tail, 0= , 0, 0); > + } else { > + /* Make sure we don't leave the master iterator pointing = to a > + * block that might get immediately consumed. > + */ > + if (rreq->io_iter.folioq =3D=3D prev && > + rreq->io_iter.folioq_slot =3D=3D prev_nr_slots) { > + rreq->io_iter.folioq =3D tail; > + rreq->io_iter.folioq_slot =3D 0; > } > - rreq->buffer_tail_slot =3D 0; > } > + rreq->buffer_tail_slot =3D 0; > + return tail; > +} > + > +/* > + * Append a folio to the rolling queue. > + */ > +int netfs_buffer_append_folio(struct netfs_io_request *rreq, struct foli= o *folio, > + bool needs_put) > +{ > + struct folio_queue *tail; > + unsigned int slot, order =3D folio_order(folio); > + > + tail =3D netfs_buffer_make_space(rreq); > + if (IS_ERR(tail)) > + return PTR_ERR(tail); > > rreq->io_iter.count +=3D PAGE_SIZE << order; > > diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c > index d32964e8ca5d..dd8241bc996b 100644 > --- a/fs/netfs/objects.c > +++ b/fs/netfs/objects.c > @@ -250,3 +250,15 @@ void netfs_put_subrequest(struct netfs_io_subrequest= *subreq, bool was_async, > if (dead) > netfs_free_subrequest(subreq, was_async); > } > + > +struct folio_queue *netfs_folioq_alloc(struct netfs_io_request *rreq, gf= p_t gfp) > +{ > + struct folio_queue *fq; > + > + fq =3D kmalloc(sizeof(*fq), gfp); > + if (fq) { > + netfs_stat(&netfs_n_folioq); > + folioq_init(fq); > + } > + return fq; > +} > diff --git a/fs/netfs/write_issue.c b/fs/netfs/write_issue.c > index 04e66d587f77..0929d9fd4ce7 100644 > --- a/fs/netfs/write_issue.c > +++ b/fs/netfs/write_issue.c > @@ -153,12 +153,22 @@ static void netfs_prepare_write(struct netfs_io_req= uest *wreq, > loff_t start) > { > struct netfs_io_subrequest *subreq; > + struct iov_iter *wreq_iter =3D &wreq->io_iter; > + > + /* Make sure we don't point the iterator at a used-up folio_queue > + * struct being used as a placeholder to prevent the queue from > + * collapsing. In such a case, extend the queue. > + */ > + if (iov_iter_is_folioq(wreq_iter) && > + wreq_iter->folioq_slot >=3D folioq_nr_slots(wreq_iter->folioq= )) { > + netfs_buffer_make_space(wreq); > + } > > subreq =3D netfs_alloc_subrequest(wreq); > subreq->source =3D stream->source; > subreq->start =3D start; > subreq->stream_nr =3D stream->stream_nr; > - subreq->io_iter =3D wreq->io_iter; > + subreq->io_iter =3D *wreq_iter; > > _enter("R=3D%x[%x]", wreq->debug_id, subreq->debug_index); > > --=20 Thanks, Steve