• Chuck Lever's avatar
    sunrpc: Fix gss_unwrap_resp_integ() again · 4047aa90
    Chuck Lever authored
    xdr_buf_read_mic() tries to find unused contiguous space in a
    received xdr_buf in order to linearize the checksum for the call
    to gss_verify_mic. However, the corner cases in this code are
    numerous and we seem to keep missing them. I've just hit yet
    another buffer overrun related to it.
    
    This overrun is at the end of xdr_buf_read_mic():
    
    1284         if (buf->tail[0].iov_len != 0)
    1285                 mic->data = buf->tail[0].iov_base + buf->tail[0].iov_len;
    1286         else
    1287                 mic->data = buf->head[0].iov_base + buf->head[0].iov_len;
    1288         __read_bytes_from_xdr_buf(&subbuf, mic->data, mic->len);
    1289         return 0;
    
    This logic assumes the transport has set the length of the tail
    based on the size of the received message. base + len is then
    supposed to be off the end of the message but still within the
    actual buffer.
    
    In fact, the length of the tail is set by the upper layer when the
    Call is encoded so that the end of the tail is actually the end of
    the allocated buffer itself. This causes the logic above to set
    mic->data to point past the end of the receive buffer.
    
    The "mic->data = head" arm of this if statement is no less fragile.
    
    As near as I can tell, this has been a problem forever. I'm not sure
    that minimizing au_rslack recently changed this pathology much.
    
    So instead, let's use a more straightforward approach: kmalloc a
    separate buffer to linearize the checksum. This is similar to
    how gss_validate() currently works.
    
    Coming back to this code, I had some trouble understanding what
    was going on. So I've cleaned up the variable naming and added
    a few comments that point back to the XDR definition in RFC 2203
    to help guide future spelunkers, including myself.
    
    As an added clean up, the functionality that was in
    xdr_buf_read_mic() is folded directly into gss_unwrap_resp_integ(),
    as that is its only caller.
    Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
    Reviewed-by: default avatarBenjamin Coddington <bcodding@redhat.com>
    Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
    4047aa90
auth_gss.c 55.8 KB