2014-06-26 14:23:17

by Avi Kivity

Subject: [PATCH] nfs: fix nfs4d readlink truncated packet

XDR requires 4-byte alignment; nfs4d READLINK reply writes out the padding,
but truncates the packet to the padding-less size.

Fix by taking the padding into consideration when truncating the packet.

Signed-off-by: Avi Kivity <[email protected]>
---
fs/nfsd/nfs4xdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 83baf2b..cba93ea 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3267,7 +3267,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd

wire_count = htonl(maxcount);
write_bytes_to_xdr_buf(xdr->buf, length_offset, &wire_count, 4);
- xdr_truncate_encode(xdr, length_offset + 4 + maxcount);
+ xdr_truncate_encode(xdr, length_offset + 4 + ALIGN(maxcount, 4));
if (maxcount & 3)
write_bytes_to_xdr_buf(xdr->buf, length_offset + 4 + maxcount,
&zero, 4 - (maxcount&3));
--
1.9.3

2014-07-02 08:40:49

by Kinglong Mee

Subject: Re: [PATCH] nfs: fix nfs4d readlink truncated packet

On 7/1/2014 19:21, Avi Kivity wrote:
> On 06/26/2014 05:23 PM, Avi Kivity wrote:
>> XDR requires 4-byte alignment; nfs4d READLINK reply writes out the padding,
>> but truncates the packet to the padding-less size.
>>
>> Fix by taking the padding into consideration when truncating the packet.
>>
>
> Ping. I should have mentioned that without this, readlink() returns an I/O error on the client.

Yes, I got
[root@localhost ~]# ll /mnt/
ls: cannot read symbolic link /mnt/test: Input/output error
total 4
-rw-r--r--. 1 root root 0 Jun 14 01:21 123456
lrwxrwxrwx. 1 root root 6 Jul 2 03:33 test
drwxr-xr-x. 1 root root 0 Jul 2 23:50 tmp
drwxr-xr-x. 1 root root 60 Jul 2 23:44 tree
[root@localhost ~]#

It's introduced by commit 476a7b1f4b2c (nfsd4: don't treat readlink like a zero-copy operation).

>
>> Signed-off-by: Avi Kivity <[email protected]>
>> ---
>> fs/nfsd/nfs4xdr.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>> index 83baf2b..cba93ea 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -3267,7 +3267,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd
>> wire_count = htonl(maxcount);
>> write_bytes_to_xdr_buf(xdr->buf, length_offset, &wire_count, 4);
>> - xdr_truncate_encode(xdr, length_offset + 4 + maxcount);
>> + xdr_truncate_encode(xdr, length_offset + 4 + ALIGN(maxcount, 4));

NFSD uses (XDR_QUADLEN(maxcount) << 2)) usually.
Anyway, it's a nice fix.

Reviewed-by: Kinglong Mee <[email protected]>

thanks,
Kinglong Mee

2014-07-01 11:21:04

by Avi Kivity

Subject: Re: [PATCH] nfs: fix nfs4d readlink truncated packet

On 06/26/2014 05:23 PM, Avi Kivity wrote:
> XDR requires 4-byte alignment; nfs4d READLINK reply writes out the padding,
> but truncates the packet to the padding-less size.
>
> Fix by taking the padding into consideration when truncating the packet.
>

Ping. I should have mentioned that without this, readlink() returns an
I/O error on the client.

> Signed-off-by: Avi Kivity <[email protected]>
> ---
> fs/nfsd/nfs4xdr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 83baf2b..cba93ea 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3267,7 +3267,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd
>
> wire_count = htonl(maxcount);
> write_bytes_to_xdr_buf(xdr->buf, length_offset, &wire_count, 4);
> - xdr_truncate_encode(xdr, length_offset + 4 + maxcount);
> + xdr_truncate_encode(xdr, length_offset + 4 + ALIGN(maxcount, 4));
> if (maxcount & 3)
> write_bytes_to_xdr_buf(xdr->buf, length_offset + 4 + maxcount,
> &zero, 4 - (maxcount&3));

2014-07-02 21:42:05

by J. Bruce Fields

Subject: Re: [PATCH] nfs: fix nfs4d readlink truncated packet

On Wed, Jul 02, 2014 at 04:40:15PM +0800, Kinglong Mee wrote:
>
> On 7/1/2014 19:21, Avi Kivity wrote:
> > On 06/26/2014 05:23 PM, Avi Kivity wrote:
> >> XDR requires 4-byte alignment; nfs4d READLINK reply writes out the padding,
> >> but truncates the packet to the padding-less size.
> >>
> >> Fix by taking the padding into consideration when truncating the packet.
> >>
> >
> > Ping. I should have mentioned that without this, readlink() returns an I/O error on the client.

Thanks, queueing up for 3.16.

> Yes, I got
> [root@localhost ~]# ll /mnt/
> ls: cannot read symbolic link /mnt/test: Input/output error
> total 4
> -rw-r--r--. 1 root root 0 Jun 14 01:21 123456
> lrwxrwxrwx. 1 root root 6 Jul 2 03:33 test
> drwxr-xr-x. 1 root root 0 Jul 2 23:50 tmp
> drwxr-xr-x. 1 root root 60 Jul 2 23:44 tree
> [root@localhost ~]#
>
> It's introduced by commit 476a7b1f4b2c (nfsd4: don't treat readlink like a zero-copy operation).
...
> >> + xdr_truncate_encode(xdr, length_offset + 4 + ALIGN(maxcount, 4));
>
> NFSD uses (XDR_QUADLEN(maxcount) << 2)) usually.

Eh, I didn't know about ALIGN()--I think I prefer it.

> Anyway, it's a nice fix.
>
> Reviewed-by: Kinglong Mee <[email protected]>

Thanks! I've also added your symptoms and the problematic commit to the
changelog.

--b.