Quoting James Morris (jmorris@xxxxxxxxx): > On Wed, 23 Jun 2010, Serge E. Hallyn wrote: > > > > + status = res.xattr_val_len; > > > + if (status <= size) > > > > res.xattr_val_len was set to size, as was status, and none of the > > 3 has been changed, so here status can't be > size can it? > > res is part of msg, and is updated by the RPC layer when decoding the > response (via xdr_decode_string_inplace()). Ah, I see, it's passed in as a member of msg. Thanks. > > Was this just a safety to prevent overrun, or did you mean to > > do some other check? (If a safety, then you'll still return > > status > size, but with garbage in value, so i think you'd want > > to also change status) > > > > > + memcpy(value, res.xattr_val, status); > > > > Yes, the check stops us from copying more than the max. expected size to > 'value'. > > It looks like we do need to return -EINVAL if the check fails, and likely > the same with listxattr(). (Or is there a better error code for reporting > invalid messages from the peer?) That'd be my pick. -serge -- 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
