linux-kernel - Re: [PATCH] scsi: use set_host_byte instead of open-coding it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Oct 2017 16:02:04 +0000
From:   Bart Van Assche <Bart.VanAssche@....com>
To:     "jthumshirn@...e.de" <jthumshirn@...e.de>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>
CC:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tj@...nel.org" <tj@...nel.org>,
        Bart Van Assche <Bart.VanAssche@....com>,
        "maier@...ux.vnet.ibm.com" <maier@...ux.vnet.ibm.com>,
        "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "oneukum@...e.com" <oneukum@...e.com>
Subject: Re: [PATCH] scsi: use set_host_byte instead of open-coding it

On Tue, 2017-10-10 at 17:29 +0200, Johannes Thumshirn wrote:
> Call set_host_byte() instead of open-coding it.
> 
> Converted using this simple Coccinelle spatch
> 
> <SmPL>
> @@
> local idexpression struct scsi_cmnd *c;
> expression E1;
> @@
> 
> - c->result = E1 << 16;
> + set_host_byte(c, E1);
> </SmPL>

This is useful but I think we should take this a step further. How
about the following approach:
- Introduce four enumeration types - one for the msg byte codes, one
  for the host byte codes, one for the driver byte codes and one for
  SCSI sense codes.
- Update the signatures of functions that store or extract these four
  bytes.
- Change the data type of scsi_cmnd.result into something that makes
  regular assignments trigger a compiler error.

I'm proposing this because there are several drivers that do not assign the
SCSI host byte correctly:
$ git grep -nH '>result = DID_' | grep -v '<< *16' | grep -v ': \*'
drivers/scsi/ips.c:953:			scsi_cmd->result = DID_ERROR;
drivers/scsi/ips.c:2657:		SC->result = DID_OK;
drivers/scsi/libiscsi.c:1731:		sc->result = DID_REQUEUE;
drivers/scsi/qla2xxx/qla_bsg.c:152:			bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:167:				bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:184:		bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:236:		bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:976:		bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:1078:		bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:1261:		bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:1375:		bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:1480:	bsg_reply->result = DID_OK;
drivers/scsi/qla2xxx/qla_bsg.c:1516:	bsg_reply->result = DID_OK;
drivers/scsi/qlogicpti.c:1049:	Cmnd->result = DID_BUS_BUSY;
drivers/scsi/stex.c:615:		cmd->result = DID_NO_CONNECT;

Bart.