On 2016-07-27 09:26, J. Bruce Fields wrote:
> On Wed, Jul 27, 2016 at 08:57:26AM +0900, Seiichi Ikarashi wrote:
>> 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?
>
> Right.
>
>>
>> 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.
>
> Oh, so if we ask the filesystem for 3 bytes it might potentially return
> those in 3 separate pages? Is that at all legal?
I think the code actually permits such a split though I am not sure that
one-single-byte-per-page situation really happens. :-)
For example, on 4kB-page architecture, 1kB-data-per-page placement will result
in 4 times larger number of pages than expected.
I do not know whether it's legal or not.
I thought NFS needs to defend itself against such large numbers,
or, can somewhere in VFS splice merge such pages into a minimum set of pages?
I cannot find such code in VFS.
Opinions or suggestions?
Seiichi.
>
>>> 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.
>
> OK, understood.
>
> --b.
>
>>
>>
>> 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
