]> git.baikalelectronics.ru Git - kernel.git/commit
sunrpc: Fix gss_unwrap_resp_integ() again
authorChuck Lever <chuck.lever@oracle.com>
Wed, 11 Mar 2020 15:21:07 +0000 (11:21 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 23 Apr 2020 08:36:41 +0000 (10:36 +0200)
commitcb4cb6cf5c9a18fcb68eedc7f36dc4a0ba7ff29c
tree85224e794ae9e8b140f9d98a1150ced4d14da92a
parentfef8dc35e008a095e9814da326e867ffee085fa2
sunrpc: Fix gss_unwrap_resp_integ() again

[ Upstream commit 537bd71b154e84052e5ffb754df7729a6ed2b829 ]

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: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
net/sunrpc/auth_gss/auth_gss.c