[PATCH] counting bug in svc_tcp_recvfrom causes panic for TCP NFS

[PATCH] counting bug in svc_tcp_recvfrom causes panic for TCP NFS

[Ted Phelps]

When svc_tcp_recvfrom() builds up its iovec to receive a packet, it
invokes page_address() with an argument which refers to
rqstp->rq_argused++.  For some memory layouts (eg not HIMEM and
not WANT_PAGE_VIRTUAL), page_address() is a macro which evalutes its
argument three times.  This results in a kernel panic when an TCP NFS
client sends a packet longer than about 1/3 of the maximum size.

The patch below causes the increment to be performed outside of the
call to page_address(), which avoids the kernel panic.  Perhaps a
better solution would be to change page_address() to be consistently
be a function for all memory layouts.

Thanks,
-Ted

--- ./linux-2.5.58-ORIG/net/sunrpc/svcsock.c	2003-01-13 22:30:06.000000000 +0000
+++ ./linux-2.5.58/net/sunrpc/svcsock.c	2003-01-15 22:42:03.000000000 +0000
@@ -924,8 +924,9 @@
 	vlen = PAGE_SIZE;
 	pnum = 1;
 	while (vlen < len) {
-		vec[pnum].iov_base = page_address(rqstp->rq_argpages[rqstp->rq_argused++]);
+		vec[pnum].iov_base = page_address(rqstp->rq_argpages[rqstp->rq_argused]);
 		vec[pnum].iov_len = PAGE_SIZE;
+		rqstp->rq_argused++;
 		pnum++;
 		vlen += PAGE_SIZE;
 	}

Re: [PATCH] counting bug in svc_tcp_recvfrom causes panic for TCP NFS

[Andrew Morton]

Ted Phelps <phelps@dstc.edu.au> wrote:
>
> Perhaps a better solution would be to change page_address() to be
> consistently be a function for all memory layouts.

Assuredly.  How about this?

diff -puN include/linux/mm.h~page_address-fix include/linux/mm.h
--- 25/include/linux/mm.h~page_address-fix	2003-01-15 15:23:45.000000000 -0800
+++ 25-akpm/include/linux/mm.h	2003-01-15 15:25:40.000000000 -0800
@@ -296,9 +296,11 @@ static inline void set_page_zone(struct 
 	page->flags |= zone_num << ZONE_SHIFT;
 }
 
-#define lowmem_page_address(page)					\
-	__va( ( ((page) - page_zone(page)->zone_mem_map)		\
-			+ page_zone(page)->zone_start_pfn) << PAGE_SHIFT)
+static inline void *lowmem_page_address(struct page *page)
+{
+	return __va(((page - page_zone(page)->zone_mem_map)
+		+ page_zone(page)->zone_start_pfn) << PAGE_SHIFT);
+}
 
 #if defined(CONFIG_HIGHMEM) && !defined(WANT_PAGE_VIRTUAL)
 #define HASHED_PAGE_VIRTUAL

_

Re: [PATCH] counting bug in svc_tcp_recvfrom causes panic for TCP NFS

[Ted Phelps]

Andrew Morton may have said:
> Ted Phelps <phelps@dstc.edu.au> wrote:
> >
> > Perhaps a better solution would be to change page_address() to be
> > consistently be a function for all memory layouts.
> 
> Assuredly.  How about this?

It no only makes sense and looks good, but it works too!

Thanks,
-Ted

Re: [PATCH] counting bug in svc_tcp_recvfrom causes panic for TCP NFS

[Andrew Morton]

Ted Phelps <phelps@dstc.edu.au> wrote:
>
> Andrew Morton may have said:
> > Ted Phelps <phelps@dstc.edu.au> wrote:
> > >
> > > Perhaps a better solution would be to change page_address() to be
> > > consistently be a function for all memory layouts.
> > 
> > Assuredly.  How about this?
> 
> It no only makes sense and looks good, but it works too!
> 

It looks like Linus has already made the same change, thanks.