Hi-
I've been auditing the server-side NFSv4 XDR encoders, and encountered
something that looks like a bug. This is nfsd4_encode_getdeviceinfo():
4675 p = xdr_reserve_space(xdr, 4);
4676 if (!p)
4677 return nfserr_resource;
4678
4679 *p++ = cpu_to_be32(gdev->gd_layout_type);
4680
4681 /* If maxcount is 0 then just update notifications */
4682 if (gdev->gd_maxcount != 0) {
4683 ops = nfsd4_layout_ops[gdev->gd_layout_type];
4684 nfserr = ops->encode_getdeviceinfo(xdr, gdev);
4685 if (nfserr) { 4686 /*
4687 * We don't bother to burden the layout drivers with
4688 * enforcing gd_maxcount, just tell the client to
4689 * come back with a bigger buffer if it's not enough.
4690 */
4691 if (xdr->buf->len + 4 > gdev->gd_maxcount)
4692 goto toosmall;
4693 return nfserr;
4694 }
4695 }
4696 4697 if (gdev->gd_notify_types) {
4698 p = xdr_reserve_space(xdr, 4 + 4);
4699 if (!p)
4700 return nfserr_resource;
4701 *p++ = cpu_to_be32(1); /* bitmap length */
4702 *p++ = cpu_to_be32(gdev->gd_notify_types);
4703 } else {
4704 p = xdr_reserve_space(xdr, 4);
4705 if (!p)
4706 return nfserr_resource;
4707 *p++ = 0;
4708 }
4709
The XDR specification looks like this:
struct device_addr4 {
layouttype4 da_layout_type;
opaque da_addr_body<>;
};
struct GETDEVICEINFO4resok {
device_addr4 gdir_device_addr;
bitmap4 gdir_notification;
};
union GETDEVICEINFO4res switch (nfsstat4 gdir_status) {
case NFS4_OK:
GETDEVICEINFO4resok gdir_resok4;
case NFS4ERR_TOOSMALL:
count4 gdir_mincount;
default:
void;
};
What concerns me is "If maxcount is 0 then just update
notifications".
When the client provides a zero gd_maxcount, then we
encode the da_layout_type field, and then appear to
skip the da_addr_body field and proceed to
encode gdir_notification field.
Seems like there's no option in the specification not to
encode da_addr_body. If gd_maxcount is zero, shouldn't we
encode a zero-length opaque for da_addr_body before encoding
gdir_notification?
I see that it's been this way since this encoder was introduced.
--
Chuck Lever
