| Message ID | 20230809221038.51296-1-nnac123@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | db17ba719bceb52f0ae4ebca0e4c17d9a3bebf05 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2,1/5] ibmvnic: Enforce stronger sanity checks on login response | expand |
Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 9 Aug 2023 17:10:34 -0500 you wrote: > Ensure that all offsets in a login response buffer are within the size > of the allocated response buffer. Any offsets or lengths that surpass > the allocation are likely the result of an incomplete response buffer. > In these cases, a full reset is necessary. > > When attempting to login, the ibmvnic device will allocate a response > buffer and pass a reference to the VIOS. The VIOS will then send the > ibmvnic device a LOGIN_RSP CRQ to signal that the buffer has been filled > with data. If the ibmvnic device does not get a response in 20 seconds, > the old buffer is freed and a new login request is sent. With 2 > outstanding requests, any LOGIN_RSP CRQ's could be for the older > login request. If this is the case then the login response buffer (which > is for the newer login request) could be incomplete and contain invalid > data. Therefore, we must enforce strict sanity checks on the response > buffer values. > > [...] Here is the summary with links: - [net,v2,1/5] ibmvnic: Enforce stronger sanity checks on login response https://git.kernel.org/netdev/net/c/db17ba719bce - [net,v2,2/5] ibmvnic: Unmap DMA login rsp buffer on send login fail https://git.kernel.org/netdev/net/c/411c565b4bc6 - [net,v2,3/5] ibmvnic: Handle DMA unmapping of login buffs in release functions https://git.kernel.org/netdev/net/c/d78a671eb899 - [net,v2,4/5] ibmvnic: Do partial reset on login failure https://git.kernel.org/netdev/net/c/23cc5f667453 - [net,v2,5/5] ibmvnic: Ensure login failure recovery is safe from other resets https://git.kernel.org/netdev/net/c/6db541ae279b You are awesome, thank you!
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 763d613adbcc..f4bb2c9ab9a4 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -5396,6 +5396,7 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, int num_tx_pools; int num_rx_pools; u64 *size_array; + u32 rsp_len; int i; /* CHECK: Test/set of login_pending does not need to be atomic @@ -5447,6 +5448,23 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, ibmvnic_reset(adapter, VNIC_RESET_FATAL); return -EIO; } + + rsp_len = be32_to_cpu(login_rsp->len); + if (be32_to_cpu(login->login_rsp_len) < rsp_len || + rsp_len <= be32_to_cpu(login_rsp->off_txsubm_subcrqs) || + rsp_len <= be32_to_cpu(login_rsp->off_rxadd_subcrqs) || + rsp_len <= be32_to_cpu(login_rsp->off_rxadd_buff_size) || + rsp_len <= be32_to_cpu(login_rsp->off_supp_tx_desc)) { + /* This can happen if a login request times out and there are + * 2 outstanding login requests sent, the LOGIN_RSP crq + * could have been for the older login request. So we are + * parsing the newer response buffer which may be incomplete + */ + dev_err(dev, "FATAL: Login rsp offsets/lengths invalid\n"); + ibmvnic_reset(adapter, VNIC_RESET_FATAL); + return -EIO; + } + size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) + be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size)); /* variable buffer sizes are not supported, so just read the
Ensure that all offsets in a login response buffer are within the size of the allocated response buffer. Any offsets or lengths that surpass the allocation are likely the result of an incomplete response buffer. In these cases, a full reset is necessary. When attempting to login, the ibmvnic device will allocate a response buffer and pass a reference to the VIOS. The VIOS will then send the ibmvnic device a LOGIN_RSP CRQ to signal that the buffer has been filled with data. If the ibmvnic device does not get a response in 20 seconds, the old buffer is freed and a new login request is sent. With 2 outstanding requests, any LOGIN_RSP CRQ's could be for the older login request. If this is the case then the login response buffer (which is for the newer login request) could be incomplete and contain invalid data. Therefore, we must enforce strict sanity checks on the response buffer values. Testing has shown that the `off_rxadd_buff_size` value is filled in last by the VIOS and will be the smoking gun for these circumstances. Until VIOS can implement a mechanism for tracking outstanding response buffers and a method for mapping a LOGIN_RSP CRQ to a particular login response buffer, the best ibmvnic can do in this situation is perform a full reset. Fixes: dff515a3e71d ("ibmvnic: Harden device login requests") Signed-off-by: Nick Child <nnac123@linux.ibm.com> Reviewed-by: Simon Horman <horms@kernel.org> --- Changes since v1: - declare variables in line length order - prevent possible infinite loop in 5th patch - add comment in 5th patch for default error handling Thanks for reviewing Jakub and Simon drivers/net/ethernet/ibm/ibmvnic.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)