[PATCH v4] nvme-tcp: fix a possible UAF when failing to allocate an io queue

[Chaitanya Kulkarni]

Yanjun,

On 3/21/23 01:36, Sagi Grimberg wrote:
> When we allocate a nvme-tcp queue, we set the data_ready callback before
> we actually need to use it. This creates the potential that if a stray
> controller sends us data on the socket before we connect, we can trigger
> the io_work and start consuming the socket.
>
> In this case reported: we failed to allocate one of the io queues, and
> as we start releasing the queues that we already allocated, we get
> a UAF [1] from the io_work which is running before it should really.
>
> Fix this by setting the socket ops callbacks only before we start the
> queue, so that we can't accidentally schedule the io_work in the
> initialization phase before the queue started. While we are at it,
> rename nvme_tcp_restore_sock_calls to pair with nvme_tcp_setup_sock_ops.
>
> One more trivial change is the CONFIG_NET_RX_BUSY_POLL check.
>
> [1]:
> [16802.107284] nvme nvme4: starting error recovery
> [16802.109166] nvme nvme4: Reconnecting in 10 seconds...
> [16812.173535] nvme nvme4: failed to connect socket: -111
> [16812.173745] nvme nvme4: Failed reconnect attempt 1
> [16812.173747] nvme nvme4: Reconnecting in 10 seconds...
> [16822.413555] nvme nvme4: failed to connect socket: -111
> [16822.413762] nvme nvme4: Failed reconnect attempt 2
> [16822.413765] nvme nvme4: Reconnecting in 10 seconds...
> [16832.661274] nvme nvme4: creating 32 I/O queues.
> [16833.919887] BUG: kernel NULL pointer dereference, address: 0000000000000088
> [16833.920068] nvme nvme4: Failed reconnect attempt 3
> [16833.920094] #PF: supervisor write access in kernel mode
> [16833.920261] nvme nvme4: Reconnecting in 10 seconds...
> [16833.920368] #PF: error_code(0x0002) - not-present page
> [16833.921086] Workqueue: nvme_tcp_wq nvme_tcp_io_work [nvme_tcp]
> [16833.921191] RIP: 0010:_raw_spin_lock_bh+0x17/0x30
> ...
> [16833.923138] Call Trace:
> [16833.923271]  <TASK>
> [16833.923402]  lock_sock_nested+0x1e/0x50
> [16833.923545]  nvme_tcp_try_recv+0x40/0xa0 [nvme_tcp]
> [16833.923685]  nvme_tcp_io_work+0x68/0xa0 [nvme_tcp]
> [16833.923824]  process_one_work+0x1e8/0x390
> [16833.923969]  worker_thread+0x53/0x3d0
> [16833.924104]  ? process_one_work+0x390/0x390
> [16833.924240]  kthread+0x124/0x150
> [16833.924376]  ? set_kthread_struct+0x50/0x50
> [16833.924518]  ret_from_fork+0x1f/0x30
> [16833.924655]  </TASK>
>
> Reported-by: Yanjun Zhang <zhangyanjun at cestc.cn>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> Changes from v3:
> - change CONFIG_NET_RX_BUSY_POLL check (Chaitanya)
>
> Changes from v2:
> - Move sock ops assignment to its own function (Hannes)
>
> Changes from v1:
> - Fix silly compliation error
>
> Yanjun, I'll be waiting for your Tested-by tag. We need it
> in order to apply this fix.
>   

Please provide a tested-by tag when you have sometime
so we can move forward with this patch to merge.

-ck