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

[Anna Schumaker <schumaker.anna@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]

Hi Krzysztof,

On Tue, Feb 14, 2023 at 6:02 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 12/02/2023 15:05, Anna Schumaker wrote:
> >>> From ac2d6c501dbcdb306480edaee625b5496f1fb4f5 Mon Sep 17 00:00:00 2001
> >>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >>> Date: Fri, 10 Feb 2023 15:50:22 -0500
> >>> Subject: [PATCH] NFSv4.2: Rework scratch handling for READ_PLUS
> >>>
> >>
> >> Patch is corrupted - maybe mail program reformatted it when sending:
> >>
> >> Applying: NFSv4.2: Rework scratch handling for READ_PLUS
> >> error: corrupt patch at line 12
> >> Patch failed at 0001 NFSv4.2: Rework scratch handling for READ_PLUS
> >
> > That's weird. I wasn't expecting gmail to reformat the patch but I
> > guess it did. I've added it as an attachment so that shouldn't happen
> > again.
>
> 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.

Thanks,
Anna

>
> [  144.690844] mmiocpy from xdr_inline_decode (net/sunrpc/xdr.c:1419 net/sunrpc/xdr.c:1454)
> [  144.695950] xdr_inline_decode from nfs4_xdr_dec_read_plus (fs/nfs/nfs42xdr.c:1063 fs/nfs/nfs42xdr.c:1147 fs/nfs/nfs42xdr.c:1360 fs/nfs/nfs42xdr.c:1341)
> [  144.702452] nfs4_xdr_dec_read_plus from call_decode (net/sunrpc/clnt.c:2595)
> [  144.708429] call_decode from __rpc_execute (include/asm-generic/bitops/generic-non-atomic.h:128 net/sunrpc/sched.c:954)
> [  144.713538] __rpc_execute from rpc_async_schedule (include/linux/sched/mm.h:336 net/sunrpc/sched.c:1035)
> [  144.719170] rpc_async_schedule from process_one_work (include/linux/jump_label.h:260 include/linux/jump_label.h:270 include/trace/events/workqueue.h:108 kernel/workqueue.c:2294)
> [  144.725238] process_one_work from worker_thread (include/linux/list.h:292 kernel/workqueue.c:2437)
> [  144.730782] worker_thread from kthread (kernel/kthread.c:378)
> [  144.735547] kthread from ret_from_fork (arch/arm/kernel/entry-common.S:149)
>
>
>
> Best regards,
> Krzysztof
>

[-- Attachment #2: aglo-read_plus.patch --]
[-- Type: text/x-patch, Size: 2860 bytes --]

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index d80ee88ca996..dee5ecf0785a 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -346,6 +346,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);
@@ -1059,6 +1060,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;
@@ -1082,6 +1084,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;
 }
 
@@ -1125,6 +1128,7 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
 	char scratch_buf[16];
 	__be32 *p;
 
+	printk("AGLO: %s START\n", __func__);
 	status = decode_op_hdr(xdr, OP_READ_PLUS);
 	if (status)
 		return status;
@@ -1158,6 +1162,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 0b0b9f1eed46..3449ea836d30 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 f7767bf22406..e1bb9fcbaad6 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1397,6 +1397,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;
@@ -1410,6 +1411,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);