Re: [PATCH v2] Prevent rqstp->rq_pages[RPCSVC_MAXPAGES] overrun

[bfields@fieldses.org (J. Bruce Fields)]

Thanks for the report.

On Tue, Jul 26, 2016 at 11:38:11AM +0900, Seiichi Ikarashi wrote:
> If over-"RPCSVC_MAXPAGES" pages are sent from file system through pipe_buffer,
> nfsd_splice_actor() corrupts struct svc_rqst and results in kernel panic. It
> actually occurred with a parallel distributed file system. It needs boundary
> checking.

This check might be useful as defensive programming, but the bug was
elsewhere.

In theory this should be prevented by the "maxcount" calculations in
nfsd4_encode_read().

What version of the kernel did you see this happen on?  What was the
client, and what was it doing?  Any other hints on reproducing?

--b.

> 
> v2: Fix semicolon-missing bug.
> 
> Signed-off-by: Seiichi Ikarashi <s.ikarashi@jp.fujitsu.com>
> 
> ---
>  fs/nfsd/vfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6fbd81e..43393f3 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -811,12 +811,20 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>  	size = sd->len;
>  
>  	if (rqstp->rq_res.page_len == 0) {
> +		if (rqstp->rq_next_page > &rqstp->rq_pages[RPCSVC_MAXPAGES-1]) {
> +			WARN_ON(1);
> +			return -ENOMEM;
> +		}
>  		get_page(page);
>  		put_page(*rqstp->rq_next_page);
>  		*(rqstp->rq_next_page++) = page;
>  		rqstp->rq_res.page_base = buf->offset;
>  		rqstp->rq_res.page_len = size;
>  	} else if (page != pp[-1]) {
> +		if (rqstp->rq_next_page > &rqstp->rq_pages[RPCSVC_MAXPAGES-1]) {
> +			WARN_ON(1);
> +			return -ENOMEM;
> +		}
>  		get_page(page);
>  		if (*rqstp->rq_next_page)
>  			put_page(*rqstp->rq_next_page);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html