1. 25 Nov, 2007 1 commit
    • Jeff Garzik's avatar
      [SCSI] NCR5380: Fix bugs and canonicalize irq handler usage · 1e641664
      Jeff Garzik authored
      * Always pass the same value to free_irq() that we pass to
        request_irq().  This fixes several bugs.
      
      * Always call NCR5380_intr() with 'irq' and 'dev_id' arguments.
      
        Note, scsi_falcon_intr() is the only case now where dev_id is not the
        scsi_host.
      
      * Always pass Scsi_Host to request_irq().  For most cases, the drivers
        already did so, and I merely neated the source code line.  In other
        cases, either NULL or a non-sensical value was passed, verified to be
        unused, then changed to be Scsi_Host in anticipation of the future.
      
      In addition to the bugs fixes, this change makes the interface usage
      consistent, which in turn enables the possibility of directly
      referencing Scsi_Host from all NCR5380_intr() invocations.
      Signed-off-by: default avatarJeff Garzik <jgarzik@redhat.com>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      1e641664
  2. 16 Nov, 2007 2 commits
    • Martin Peschke's avatar
      [SCSI] zfcp: fix cleanup of dismissed error recovery actions · 86e8dfc5
      Martin Peschke authored
      Calling zfcp_erp_strategy_check_action() after zfcp_erp_action_to_running()
      in zfcp_erp_strategy() might cause an unbalanced up() for erp_ready_sem,
      which makes the zfcp recovery fail somewhere along the way:
      
      erp thread processing erp_action:
      |
      |	someone waking up erp thread for erp_action
      |	|
      |	|		someone else dismissing erp_action:
      |	|		|
      V	V		V
      
      	write_lock_irqsave(&adapter->erp_lock, flags);
      	...
      	if (zfcp_erp_action_exists(erp_action) == ZFCP_ERP_ACTION_RUNNING) {
      		zfcp_erp_action_to_ready(erp_action);
      		up(&adapter->erp_ready_sem);	/* first up() for erp_action */
      	}
      	write_unlock_irqrestore(&adapter->erp_lock, flags);
      
      write_lock_irqsave(&adapter->erp_lock, flags);
      ...
      zfcp_erp_action_to_running(erp_action);
      write_unlock_restore(&adapter->erp_lock, flags);
      /* processing erp_action */
      
      			write_lock_irqsave(&adapter->erp_lock, flags);
      			...
      			erp_action->status |= ZFCP_STATUS_ERP_DISMISSED;
      			if (zfcp_erp_action_exists(erp_action) ==
      						ZFCP_ERP_ACTION_RUNNING) {
      				zfcp_erp_action_to_ready(erp_action);
      				up(&adapter->erp_ready_sem);
      				/* second, unbalanced up() for erp_action */
      			}
      			...
      			write_unlock_restore(&adapter->erp_lock, flags);
      
      write_lock_irqsave(&adapter->erp_lock, flags);
      if (erp_action->status & ZFCP_STATUS_ERP_DISMISSED) {
      	zfcp_erp_action_dequeue(erp_action);
      	retval = ZFCP_ERP_DISMISSED;
      }
      ...
      write_unlock_restore(&adapter->erp_lock, flags);
      down(&adapter->erp_ready_sem);
      /* this down() is meant to balance the first up() */
      
      The erp thread must not dismiss an erp_action after moving that action to
      erp_running_head. Instead it should just go through the down() operation,
      which balances the first up(), and run through zfcp_erp_strategy one more
      time for the second up(), which eventually cleans up erp_action. Which
      is similar to the normal processing of an event for erp_action doing
      something asynchronously (e.g. waiting for the completion of an fsf_req).
      
      This only works if we make sure that a dismissed erp_action is passed to
      zfcp_erp_strategy() prior to the other action, which caused actions to be
      dismissed. Therefore the patch implements this rule: running actions go to
      the head of the ready list; new actions go to the tail of the ready list;
      the erp thread picks actions to be processed from the ready list's head.
      Signed-off-by: default avatarMartin Peschke <mp3@de.ibm.com>
      Acked-by: default avatarSwen Schillig <swen@vnet.ibm.com>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      86e8dfc5
    • Martin Peschke's avatar
      [SCSI] zfcp: fix dismissal of error recovery actions · d0076f77
      Martin Peschke authored
      zfcp_erp_action_dismiss() used to ignore any actions in the ready list. This
      is a bug. Any action superseded by a stronger action needs to be dismissed.
      This patch changes zfcp_erp_action_dismiss() so that it dismisses actions
      regardless of their list affiliation. The ERP thread is able to handle this.
      It is important to kick the erp thread only for actions in the running list,
      though, as an imbalance of wakeup signals would confuse the erp thread
      otherwise.
      Signed-off-by: default avatarMartin Peschke <mp3@de.ibm.com>
      Acked-by: default avatarSwen Schillig <swen@vnet.ibm.com>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      d0076f77
  3. 15 Nov, 2007 1 commit
  4. 14 Nov, 2007 2 commits
    • Tony Battersby's avatar
      [SCSI] iscsi: return data transfer residual for data-out commands · 6ee6a2f0
      Tony Battersby authored
      Currently, the iSCSI driver returns the data transfer residual for
      data-in commands (e.g. read) but not data-out commands (e.g. write).
      This patch makes it return the data transfer residual for both types of
      commands.
      Signed-off-by: default avatarTony Battersby <tonyb@cybernetics.com>
      Signed-off-by: default avatarMike Christie <michaelc@cs.wisc.edu>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      6ee6a2f0
    • Tony Battersby's avatar
      [SCSI] iscsi_tcp: fix potential lockup with write commands · 505f76b3
      Tony Battersby authored
      There is a race condition in iscsi_tcp.c that may cause it to forget
      that it received a R2T from the target.  This race may cause a data-out
      command (such as a write) to lock up.  The race occurs here:
      
      static int
      iscsi_send_unsol_pdu(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
      {
      	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
      	int rc;
      
      	if (tcp_ctask->xmstate & XMSTATE_UNS_HDR) {
      		BUG_ON(!ctask->unsol_count);
      		tcp_ctask->xmstate &= ~XMSTATE_UNS_HDR; <---- RACE
      		...
      
      static int
      iscsi_r2t_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
      {
      	...
      	tcp_ctask->xmstate |= XMSTATE_SOL_HDR_INIT; <---- RACE
      	...
      
      While iscsi_xmitworker() (called from scsi_queue_work()) is preparing to
      send unsolicited data, iscsi_tcp_data_recv() (called from
      tcp_read_sock()) interrupts it upon receipt of a R2T from the target.
      Both contexts do read-modify-write of tcp_ctask->xmstate.  Usually, gcc
      on x86 will make &= and |= atomic on UP (not guaranteed of course), but
      in this case iscsi_send_unsol_pdu() reads the value of xmstate before
      clearing the bit, which causes gcc to read xmstate into a CPU register,
      test it, clear the bit, and then store it back to memory.  If the recv
      interrupt happens during this sequence, then the XMSTATE_SOL_HDR_INIT
      bit set by the recv interrupt will be lost, and the R2T will be
      forgotten.
      
      The patch below (against 2.6.24-rc1) converts accesses of xmstate to use
      set_bit, clear_bit, and test_bit instead of |= and &=.  I have tested
      this patch and verified that it fixes the problem.  Another possible
      approach would be to hold a lock during most of the rx/tx setup and
      post-processing, and drop the lock only for the actual rx/tx.
      Signed-off-by: default avatarTony Battersby <tonyb@cybernetics.com>
      Signed-off-by: default avatarMike Christie <michaelc@cs.wisc.edu>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      505f76b3
  5. 11 Nov, 2007 1 commit
    • Alan Cox's avatar
      [SCSI] aacraid: fix security weakness · 5f78e89b
      Alan Cox authored
      Actually there are several but one is trivially fixed
      
      1.	FSACTL_GET_NEXT_ADAPTER_FIB ioctl does not lock dev->fib_list
      but needs to
      2.	Ditto for FSACTL_CLOSE_GET_ADAPTER_FIB
      3.	It is possible to construct an attack via the SRB ioctls where
      the user obtains assorted elevated privileges. Various approaches are
      possible, the trivial ones being things like writing to the raw media
      via scsi commands and the swap image of other executing programs with
      higher privileges.
      
      So the ioctls should be CAP_SYS_RAWIO - at least all the FIB manipulating
      ones. This is a bandaid fix for #3 but probably the ioctls should grow
      their own capable checks. The other two bugs need someone competent in that
      driver to fix them.
      Signed-off-by: default avatarAlan Cox <alan@redhat.com>
      Acked-by: default avatarMark Salyzyn <mark_salyzyn@adaptec.com>
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      5f78e89b
  6. 07 Nov, 2007 3 commits
  7. 06 Nov, 2007 13 commits
  8. 05 Nov, 2007 17 commits