Message-ID: <e09daca61c29b70fcc990896c78ecdfd06532411.camel@kernel.org>
Subject: Re: [PATCH v1 0/3] rq_pages bounds checking
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Chuck Lever <cel@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        "dcritch@redhat.com" <dcritch@redhat.com>,
        "d.lesca@solinos.it" <d.lesca@solinos.it>
Date:   Sat, 18 Mar 2023 12:37:54 -0400
In-Reply-To: <8356EDBD-7062-4F44-87F1-FF2BAE7F359F@oracle.com>
References: <167909365790.1672.13118429954916842449.stgit@klimt.1015granger.net>
         <cf2ad1349a19b472653c500cc7e287843a0cb8c7.camel@kernel.org>
         <8356EDBD-7062-4F44-87F1-FF2BAE7F359F@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Sat, 2023-03-18 at 15:20 +0000, Chuck Lever III wrote:
>=20
> > On Mar 18, 2023, at 6:04 AM, Jeff Layton <jlayton@kernel.org> wrote:
> >=20
> > On Fri, 2023-03-17 at 19:01 -0400, Chuck Lever wrote:
> > > A slightly modified take on Jeff's earlier patches, tested with
> > > both NFSv3 and NFSv4.1 via simple fault injection in
> > > svc_rqst_replace_page().
> > >=20
> > > In general I'm in favor of more rq_pages bounds checking by
> > > replacing direct modification of the rq_respages and rq_next_page
> > > fields with accessor functions.
> > >=20
> > > ---
> > >=20
> > > Chuck Lever (2):
> > >      SUNRPC: add bounds checking to svc_rqst_replace_page
> > >      NFSD: Watch for rq_pages bounds checking errors in nfsd_splice_a=
ctor()
> > >=20
> > > Jeff Layton (1):
> > >      nfsd: don't replace page in rq_pages if it's a continuation of l=
ast page
> > >=20
> > >=20
> > > fs/nfsd/vfs.c                 | 15 +++++++++++++--
> > > include/linux/sunrpc/svc.h    |  2 +-
> > > include/trace/events/sunrpc.h | 25 +++++++++++++++++++++++++
> > > net/sunrpc/svc.c              | 15 ++++++++++++++-
> > > 4 files changed, 53 insertions(+), 4 deletions(-)
> > >=20
> > > --
> > > Chuck Lever
> > >=20
> >=20
> > Looks good, Chuck, thanks. You can add this to the last two:
> >=20
> > Reviewed-by: Jeff Layton <jlayton@kernel.org>
>=20
> Excellent, thanks!
>=20
> When I started I expected 3/3 to be more substantial, but since it's
> just a handful of lines and the patch descriptions are about the same,
> I'm going to squash 2/3 and 3/3 together.
>=20
> Only question is whether to apply that to nfsd-next or nfsd-fixes.
> Since it's a defensive change, I was thinking nfsd-next. Let me know
> if you think it should get merged sooner.
>=20

No, that sounds fine for those. Patch #1 needs to go in ASAP, of course.
--=20
Jeff Layton <jlayton@kernel.org>