Re: Regression: NULL pointer dereference after NFS_V4_2_READ_PLUS (commit 7fd461c47)

[Anna Schumaker]

On Thu, Jun 15, 2023 at 1:04 PM Anna Schumaker <schumaker.anna@xxxxxxxxx> wrote:
>
> On Thu, Jun 15, 2023 at 9:01 AM Anna Schumaker <schumaker.anna@xxxxxxxxx> wrote:
> >
> > On Thu, Jun 15, 2023 at 4:55 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> > >
> > > On 15/06/2023 10:52, Krzysztof Kozlowski wrote:
> > > > On 14/06/2023 22:55, Anna Schumaker wrote:
> > > >>>>> Still null ptr (built on 420b2d4 with your patch):
> > > >>>>
> > > >>>> We're through the merge window and at rc1 now, so I can spend more
> > > >>>> time scratching my head over your bug again. We've come up with a
> > > >>>> patch (attached) that adds a bunch of printks to show us what the
> > > >>>> kernel thinks is going on. Do you mind trying it out and letting us
> > > >>>> know what gets printed out? You'll need to make sure
> > > >>>> CONFIG_NFS_V4_2_READ_PLUS is enabled when compiling the kernel.
> > > >>>
> > > >>> The patch does not apply. I tried: v6.4-rc1, v6.4-rc5, next-20230609.
> > > >>
> > > >> Can you try the attached patch on top of my 3-patch series from the
> > > >> other day, and let me know what gets printed out? It adds a bunch of
> > > >> printk()s at strategic points to print out what is going on with the
> > > >> xdr scratch buffer since it's suddenly a bad memory address after
> > > >> working for a bit on your machine.
> > > >>
> > > >
> > > > Here you have entire log - attached (113 kB, I hope goes past mailing
> > > > lists/spam filters).
> > >
> > > As expected this bounced from the mailing lists, but I hope you got it.
> > > If not, let me know.
> >
> > I did still receive it. Thanks!
>
> Can you swap out yesterday's patch with this patch? I've adjusted what
> gets printed out, and added printk()s to xdr_copy_to_scratch().  I'm
> starting to think that the xdr scratch buffer is fine, and that it's
> the other pointer passed to memcpy() in that function that's the
> problem, and the output from this patch will confirm for me.

Oh, and can you add this one on top of the v2 patch as well?

Thanks,
Anna

>
> Thanks,
> Anna
>
> >
> > Anna
> > >
> > > Best regards,
> > > Krzysztof
> > >
From 8252138376a2ba3e53807fafab9c382b6acf503f Mon Sep 17 00:00:00 2001
From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
Date: Thu, 15 Jun 2023 13:13:47 -0400
Subject: [PATCH] NFS: Olga's prink() patch

Adds a handful of printk() statements to print out the state of the xdr
stream.
---
 fs/nfs/nfs42xdr.c | 5 +++++
 net/sunrpc/clnt.c | 1 +
 net/sunrpc/xdr.c  | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 6a4eade2400b..22628057d080 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -352,6 +352,7 @@ static void encode_read_plus(struct xdr_stream *xdr,
 			     const struct nfs_pgio_args *args,
 			     struct compound_hdr *hdr)
 {
+	printk("AGLO: %s setting up decode buffer size=%d\n", __func__, decode_read_plus_maxsz);
 	encode_op_hdr(xdr, OP_READ_PLUS, decode_read_plus_maxsz, hdr);
 	encode_nfs4_stateid(xdr, &args->stateid);
 	encode_uint64(xdr, args->offset);
@@ -1065,6 +1066,7 @@ static int decode_read_plus_segment(struct xdr_stream *xdr,
 {
 	__be32 *p;
 
+	printk("AGLO: %s start seg=%p\n", __func__, seg);
 	p = xdr_inline_decode(xdr, 4);
 	if (!p)
 		return -EIO;
@@ -1088,6 +1090,7 @@ static int decode_read_plus_segment(struct xdr_stream *xdr,
 		xdr_decode_hyper(p, &seg->hole.length);
 	} else
 		return -EINVAL;
+	printk("AGLO: %s end seg=%p xdr->nwords=%d\n", __func__, seg, xdr->nwords);
 	return 0;
 }
 
@@ -1130,6 +1133,7 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
 	int status, i;
 	__be32 *p;
 
+	printk("AGLO: %s START\n", __func__);
 	status = decode_op_hdr(xdr, OP_READ_PLUS);
 	if (status)
 		return status;
@@ -1161,6 +1165,7 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
 
 out:
 	kfree(segs);
+	printk("AGLO: %s END\n", __func__);
 	return status;
 }
 
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index d2ee56634308..4426a25d0152 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1337,6 +1337,7 @@ void rpc_prepare_reply_pages(struct rpc_rqst *req, struct page **pages,
 {
 	hdrsize += RPC_REPHDRSIZE + req->rq_cred->cr_auth->au_ralign;
 
+	printk("AGLO: %s hdrsize %d (<<2 %d) HDRSIZE %d auligh %d\n", __func__, hdrsize, hdrsize << 2, RPC_REPHDRSIZE, req->rq_cred->cr_auth->au_ralign);
 	xdr_inline_pages(&req->rq_rcv_buf, hdrsize << 2, pages, base, len);
 	trace_rpc_xdr_reply_pages(req->rq_task, &req->rq_rcv_buf);
 }
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 37c8f7e519dd..7f635f601dd4 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1422,6 +1422,7 @@ static __be32 * __xdr_inline_decode(struct xdr_stream *xdr, size_t nbytes)
 	__be32 *p = xdr->p;
 	__be32 *q = p + nwords;
 
+	printk("AGLO: %s nwords=%d xdr->nwords=%d q=%p xdr->end=%p p=%p (%d %d)\n", __func__, nwords, xdr->nwords, q, xdr->end, p, q > xdr->end, q < p);
 	if (unlikely(nwords > xdr->nwords || q > xdr->end || q < p))
 		return NULL;
 	xdr->p = q;
@@ -1435,6 +1436,7 @@ static __be32 *xdr_copy_to_scratch(struct xdr_stream *xdr, size_t nbytes)
 	char *cpdest = xdr->scratch.iov_base;
 	size_t cplen = (char *)xdr->end - (char *)xdr->p;
 
+	printk("AGLO: %s here\n", __func__);
 	if (nbytes > xdr->scratch.iov_len)
 		goto out_overflow;
 	p = __xdr_inline_decode(xdr, cplen);
-- 
2.41.0