linux-stable.git/drivers/infiniband/ulp, branch v3.16.78

IB/ipoib: Fix for use-after-free in ipoib_cm_tx_start

2019-05-02T20:41:29+00:00

commit 6ab4aba00f811a5265acc4d3eb1863bb3ca60562 upstream.

The following BUG was reported by kasan:

 BUG: KASAN: use-after-free in ipoib_cm_tx_start+0x430/0x1390 [ib_ipoib]
 Read of size 80 at addr ffff88034c30bcd0 by task kworker/u16:1/24020

 Workqueue: ipoib_wq ipoib_cm_tx_start [ib_ipoib]
 Call Trace:
  dump_stack+0x9a/0xeb
  print_address_description+0xe3/0x2e0
  kasan_report+0x18a/0x2e0
  ? ipoib_cm_tx_start+0x430/0x1390 [ib_ipoib]
  memcpy+0x1f/0x50
  ipoib_cm_tx_start+0x430/0x1390 [ib_ipoib]
  ? kvm_clock_read+0x1f/0x30
  ? ipoib_cm_skb_reap+0x610/0x610 [ib_ipoib]
  ? __lock_is_held+0xc2/0x170
  ? process_one_work+0x880/0x1960
  ? process_one_work+0x912/0x1960
  process_one_work+0x912/0x1960
  ? wq_pool_ids_show+0x310/0x310
  ? lock_acquire+0x145/0x440
  worker_thread+0x87/0xbb0
  ? process_one_work+0x1960/0x1960
  kthread+0x314/0x3d0
  ? kthread_create_worker_on_cpu+0xc0/0xc0
  ret_from_fork+0x3a/0x50

 Allocated by task 0:
  kasan_kmalloc+0xa0/0xd0
  kmem_cache_alloc_trace+0x168/0x3e0
  path_rec_create+0xa2/0x1f0 [ib_ipoib]
  ipoib_start_xmit+0xa98/0x19e0 [ib_ipoib]
  dev_hard_start_xmit+0x159/0x8d0
  sch_direct_xmit+0x226/0xb40
  __dev_queue_xmit+0x1d63/0x2950
  neigh_update+0x889/0x1770
  arp_process+0xc47/0x21f0
  arp_rcv+0x462/0x760
  __netif_receive_skb_core+0x1546/0x2da0
  netif_receive_skb_internal+0xf2/0x590
  napi_gro_receive+0x28e/0x390
  ipoib_ib_handle_rx_wc_rss+0x873/0x1b60 [ib_ipoib]
  ipoib_rx_poll_rss+0x17d/0x320 [ib_ipoib]
  net_rx_action+0x427/0xe30
  __do_softirq+0x28e/0xc42

 Freed by task 26680:
  __kasan_slab_free+0x11d/0x160
  kfree+0xf5/0x360
  ipoib_flush_paths+0x532/0x9d0 [ib_ipoib]
  ipoib_set_mode_rss+0x1ad/0x560 [ib_ipoib]
  set_mode+0xc8/0x150 [ib_ipoib]
  kernfs_fop_write+0x279/0x440
  __vfs_write+0xd8/0x5c0
  vfs_write+0x15e/0x470
  ksys_write+0xb8/0x180
  do_syscall_64+0x9b/0x420
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

 The buggy address belongs to the object at ffff88034c30bcc8
                which belongs to the cache kmalloc-512 of size 512
 The buggy address is located 8 bytes inside of
                512-byte region [ffff88034c30bcc8, ffff88034c30bec8)
 The buggy address belongs to the page:

The following race between change mode and xmit flow is the reason for
this use-after-free:

Change mode     Send packet 1 to GID XX      Send packet 2 to GID XX
     |                    |                             |
   start                  |                             |
     |                    |                             |
     |                    |                             |
     |         Create new path for GID XX               |
     |           and update neigh path                  |
     |                    |                             |
     |                    |                             |
     |                    |                             |
 flush_paths              |                             |
                          |                             |
               queue_work(cm.start_task)                |
                          |                 Path for GID XX not found
                          |                      create new path
                          |
                          |
               start_task runs with old
                    released path

There is no locking to protect the lifetime of the path through the
ipoib_cm_tx struct, so delete it entirely and always use the newly looked
up path under the priv->lock.

Fixes: 546481c2816e ("IB/ipoib: Fix memory corruption in ipoib cm mode connect flow")
Signed-off-by: Feras Daoud 
Reviewed-by: Erez Shitrit 
Signed-off-by: Leon Romanovsky 
Signed-off-by: Jason Gunthorpe 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

iser: set sector for ambiguous mr status errors

2019-02-11T17:54:08+00:00

commit 24c3456c8d5ee6fc1933ca40f7b4406130682668 upstream.

If for some reason we failed to query the mr status, we need to make sure
to provide sufficient information for an ambiguous error (guard error on
sector 0).

Fixes: 0a7a08ad6f5f ("IB/iser: Implement check_protection")
Reported-by: Dan Carpenter 
Signed-off-by: Sagi Grimberg 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Ben Hutchings

IB/ipoib: Avoid a race condition between start_xmit and cm_rep_handler

2018-12-16T22:09:09+00:00

commit 816e846c2eb9129a3e0afa5f920c8bbc71efecaa upstream.

Inside of start_xmit() the call to check if the connection is up and the
queueing of the packets for later transmission is not atomic which leaves
a window where cm_rep_handler can run, set the connection up, dequeue
pending packets and leave the subsequently queued packets by start_xmit()
sitting on neigh->queue until they're dropped when the connection is torn
down. This only applies to connected mode. These dropped packets can
really upset TCP, for example, and cause multi-minute delays in
transmission for open connections.

Here's the code in start_xmit where we check to see if the connection is
up:

       if (ipoib_cm_get(neigh)) {
               if (ipoib_cm_up(neigh)) {
                       ipoib_cm_send(dev, skb, ipoib_cm_get(neigh));
                       goto unref;
               }
       }

The race occurs if cm_rep_handler execution occurs after the above
connection check (specifically if it gets to the point where it acquires
priv->lock to dequeue pending skb's) but before the below code snippet in
start_xmit where packets are queued.

       if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
               push_pseudo_header(skb, phdr->hwaddr);
               spin_lock_irqsave(&priv->lock, flags);
               __skb_queue_tail(&neigh->queue, skb);
               spin_unlock_irqrestore(&priv->lock, flags);
       } else {
               ++dev->stats.tx_dropped;
               dev_kfree_skb_any(skb);
       }

The patch acquires the netif tx lock in cm_rep_handler for the section
where it sets the connection up and dequeues and retransmits deferred
skb's.

Fixes: 839fcaba355a ("IPoIB: Connected mode experimental support")
Signed-off-by: Aaron Knister 
Tested-by: Ira Weiny 
Reviewed-by: Ira Weiny 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Ben Hutchings

IB/IPoIB: Set ah valid flag in multicast send flow

2018-12-16T22:08:32+00:00

commit 75da96067ade4e7854379ec2f7834f3497652b1a upstream.

The change of ipoib_ah data structure with adding "valid" flag and
checks of ah->valid in ipoib_start_xmit affected multicast packet flow.

Since the multicast flow doesn't invoke path_rec_start, "ah->valid" flag
remains unset, so that ipoib_start_xmit end up with neigh_refresh_path
instead of sending the packet using neigh.

"ah->valid" has to be set in multicast send flow. As a result IPoIB
starts sending packets via neigh immediately and eliminates 60sec delay
of neigh keep alive interval.

The typical example of this issue are two sequential arpings:

arping 11.134.208.9 -> got response (mcast_send)
arping 11.134.208.9 -> no response  (ah->valid = 0)

Fixes: fa9391dbad4b ("RDMA/ipoib: Update paths on CLIENT_REREG/SM_CHANGE events")
Signed-off-by: Denis Drozdov 
Reviewed-by: Erez Shitrit 
Reviewed-by: Feras Daoud 
Signed-off-by: Leon Romanovsky 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Ben Hutchings

IB/srpt: Support HCAs with more than two ports

2018-12-16T22:08:07+00:00

commit e620ebfc228dcbef7519e3d16f43c6c6f1a1d0cb upstream.

Since there are adapters that have four ports, increase the size of
the srpt_device.port[] array. This patch avoids that the following
warning is hit with quad port Chelsio adapters:

    WARN_ON(sdev->device->phys_port_cnt > ARRAY_SIZE(sdev->port));

Reported-by: Steve Wise 
Signed-off-by: Bart Van Assche 
Cc: Steve Wise 
Cc: Christoph Hellwig 
Reviewed-by: Steve Wise 
Signed-off-by: Jason Gunthorpe 
[bwh: Backported to 3.16:
 - Use inline calculation instead of struct_size; the number of ports is not
   user-controlled so no overflow check is needed
 - Adjust context]
Signed-off-by: Ben Hutchings

IB/isert: fix T10-pi check mask setting

2018-11-20T18:05:08+00:00

commit 0e12af84cdd3056460f928adc164f9e87f4b303b upstream.

A copy/paste bug (probably) caused setting of an app_tag check mask
in case where a ref_tag check was needed.

Fixes: 38a2d0d429f1 ("IB/isert: convert to the generic RDMA READ/WRITE API")
Fixes: 9e961ae73c2c ("IB/isert: Support T10-PI protected transactions")
Reviewed-by: Christoph Hellwig 
Reviewed-by: Sagi Grimberg 
Reviewed-by: Martin K. Petersen 
Signed-off-by: Max Gurtovoy 
Signed-off-by: Jason Gunthorpe 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

IB/isert: Fix for lib/dma_debug check_sync warning

2018-11-20T18:05:02+00:00

commit 763b69654bfb88ea3230d015e7d755ee8339f8ee upstream.

The following error message occurs on a target host in a debug build
during session login:

[ 3524.411874] WARNING: CPU: 5 PID: 12063 at lib/dma-debug.c:1207 check_sync+0x4ec/0x5b0
[ 3524.421057] infiniband hfi1_0: DMA-API: device driver tries to sync DMA memory it has not allocated [device address=0x0000000000000000] [size=76 bytes]
......snip .....

[ 3524.535846] CPU: 5 PID: 12063 Comm: iscsi_np Kdump: loaded Not tainted 3.10.0-862.el7.x86_64.debug #1
[ 3524.546764] Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.2.6 06/08/2015
[ 3524.555740] Call Trace:
[ 3524.559102]  [] dump_stack+0x19/0x1b
[ 3524.565477]  [] __warn+0xd8/0x100
[ 3524.571557]  [] warn_slowpath_fmt+0x5f/0x80
[ 3524.578610]  [] check_sync+0x4ec/0x5b0
[ 3524.585177]  [] ? set_cpus_allowed_ptr+0x5f/0x1c0
[ 3524.592812]  [] debug_dma_sync_single_for_cpu+0x80/0x90
[ 3524.601029]  [] ? x2apic_send_IPI_mask+0x13/0x20
[ 3524.608574]  [] ? native_smp_send_reschedule+0x5b/0x80
[ 3524.616699]  [] ? resched_curr+0xf6/0x140
[ 3524.623567]  [] isert_create_send_desc.isra.26+0xe0/0x110 [ib_isert]
[ 3524.633060]  [] isert_put_login_tx+0x55/0x8b0 [ib_isert]
[ 3524.641383]  [] ? try_to_wake_up+0x1a4/0x430
[ 3524.648561]  [] iscsi_target_do_tx_login_io+0xdd/0x230 [iscsi_target_mod]
[ 3524.658557]  [] iscsi_target_do_login+0x1a7/0x600 [iscsi_target_mod]
[ 3524.668084]  [] ? kstrdup+0x49/0x60
[ 3524.674420]  [] iscsi_target_start_negotiation+0x56/0xc0 [iscsi_target_mod]
[ 3524.684656]  [] __iscsi_target_login_thread+0x90e/0x1070 [iscsi_target_mod]
[ 3524.694901]  [] ? __iscsi_target_login_thread+0x1070/0x1070 [iscsi_target_mod]
[ 3524.705446]  [] ? __iscsi_target_login_thread+0x1070/0x1070 [iscsi_target_mod]
[ 3524.715976]  [] iscsi_target_login_thread+0x28/0x60 [iscsi_target_mod]
[ 3524.725739]  [] kthread+0xef/0x100
[ 3524.732007]  [] ? insert_kthread_work+0x80/0x80
[ 3524.739540]  [] ret_from_fork_nospec_begin+0x21/0x21
[ 3524.747558]  [] ? insert_kthread_work+0x80/0x80
[ 3524.755088] ---[ end trace 23f8bf9238bd1ed8 ]---
[ 3595.510822] iSCSI/iqn.1994-05.com.redhat:537fa56299: Unsupported SCSI Opcode 0xa3, sending CHECK_CONDITION.

The code calls dma_sync on login_tx_desc->dma_addr prior to initializing it
with dma-mapped address.
login_tx_desc is a part of iser_conn structure and is used only once
during login negotiation, so the issue is fixed by eliminating
dma_sync call for this buffer using a special case routine.

Reviewed-by: Mike Marciniszyn 
Reviewed-by: Don Dutile 
Signed-off-by: Alex Estrin 
Signed-off-by: Dennis Dalessandro 
Signed-off-by: Doug Ledford 
[bwh: Backported to 3.16:
 - Parameters to isert_create_send_desc() are not redundant; forward them
   all to __isert_create_send_desc()
 - Adjust context]
Signed-off-by: Ben Hutchings

RDMA/ipoib: Update paths on CLIENT_REREG/SM_CHANGE events

2018-11-20T18:05:00+00:00

commit fa9391dbad4b868512ed22a7e41765f881a8a935 upstream.

We do a light flush on CLIENT_REREG and SM_CHANGE events.  This goes
through and marks paths invalid. But we weren't always checking for this
validity when we needed to, and so we could keep using a path marked
invalid.  What's more, once we establish a path with a valid ah, we put
a pointer to the ah in the neigh struct directly, so even if we mark the
path as invalid, as long as the neigh has a direct pointer to the ah, it
keeps using the old, outdated ah.

To fix this we do several things.

1) Put the valid flag in the ah instead of the path struct, so when we
put the ah pointer directly in the neigh struct, we can easily check the
validity of the ah on send events.
2) Check the neigh->ah and neigh->ah->valid elements in the needed
places, and if we have an ah, but it's invalid, then invoke a refresh of
the ah.
3) Fix the various places that check for path, but didn't check for
path->valid (now path->ah && path->ah->valid).

Reported-by: Evgenii Smirnov 
Fixes: ee1e2c82c245 ("IPoIB: Refresh paths instead of flushing them on SM change events")
Signed-off-by: Doug Ledford 
[bwh: Backported to 3.16:
 - s/phdr->hwaddr/cb->hdwaddr/
 - s/ipoib_priv/netdev_priv/
 - Adjust context]
Signed-off-by: Ben Hutchings

IB/srp: Fix srp_abort()

2018-10-21T07:44:51+00:00

commit e68088e78d82920632eba112b968e49d588d02a2 upstream.

Before commit e494f6a72839 ("[SCSI] improved eh timeout handler") it
did not really matter whether or not abort handlers like srp_abort()
called .scsi_done() when returning another value than SUCCESS. Since
that commit however this matters. Hence only call .scsi_done() when
returning SUCCESS.

Signed-off-by: Bart Van Assche 
Signed-off-by: Jason Gunthorpe 
[bwh: Backported to 3.16: s/ch/target/]
Signed-off-by: Ben Hutchings

IB/ipoib: Do not warn if IPoIB debugfs doesn't exist

2018-06-16T21:22:16+00:00

commit 14fa91e0fef8e4d6feb8b1fa2a807828e0abe815 upstream.

netdev_wait_allrefs() could rebroadcast NETDEV_UNREGISTER event
multiple times until all refs are gone, which will result in calling
ipoib_delete_debug_files multiple times and printing a warning.

Remove the WARN_ONCE since checks of NULL pointers before calling
debugfs_remove are not needed.

Fixes: 771a52584096 ("IB/IPoIB: ibX: failed to create mcg debug file")
Signed-off-by: Alaa Hleihel 
Signed-off-by: Leon Romanovsky 
Reviewed-by: Dennis Dalessandro 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Ben Hutchings