Hi Bruce,
On 2016-07-27 05:19, J. Bruce Fields wrote:
> 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.
Yah, I think the main factor exists in file system and/or VFS splice sides.
But I also think that NFS should defend itself against overlimit pages
because the limit is decided by NFS/SUNRPC, not by file system and others.
>
> In theory this should be prevented by the "maxcount" calculations in
> nfsd4_encode_read().
The "maxcount" looks just limiting the read length from the file system.
Is my understanding correct?
And the number of pages provided from the file system is up to the file system.
The file system can split the read data into an arbitrary number of pages.
>
> 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?
It was a NFSv3 read access from a client of maybe-Linux-based system
against the server of RHEL6 system exporting a file system that I cannot
access its source code. Sorry for lack of info.
Seiichi.
>
> --b.
>
>>
>> v2: Fix semicolon-missing bug.
>>
>> Signed-off-by: Seiichi Ikarashi <s.ikarashi@xxxxxxxxxxxxxx>
>>
>> ---
>> 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@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
