linux-kernel - Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg

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]

Message-ID: <20161103174250.ury2u5zbyi2tfyvt@linux-x5ow.site>
Date:   Thu, 3 Nov 2016 18:42:50 +0100
From:   Johannes Thumshirn <jthumshirn@...e.de>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Steffen Maier <maier@...ux.vnet.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        Hannes Reinecke <hare@...e.de>,
        Linux Kernel Mailinglist <linux-kernel@...r.kernel.org>,
        Linux SCSI Mailinglist <linux-scsi@...r.kernel.org>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Anil Gurumurthy <anil.gurumurthy@...gic.com>,
        Sudarsana Kalluru <sudarsana.kalluru@...gic.com>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Johannes Thumshirn <jth@...nel.org>,
        James Smart <james.smart@...gotech.com>,
        Dick Kennedy <dick.kennedy@...gotech.com>,
        "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" 
        <qla2xxx-upstream@...gic.com>,
        "open list:S390 ZFCP DRIVER" <linux-s390@...r.kernel.org>,
        "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" 
        <linuxppc-dev@...ts.ozlabs.org>,
        "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" 
        <fcoe-devel@...n-fcoe.org>
Subject: Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and
 fc_bsg_job::reply directly

On Thu, Nov 03, 2016 at 08:17:51AM -0700, Christoph Hellwig wrote:
> On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote:
> > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote:
> > > Hm, still behaves for me like I reported for v2:
> > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2
> > 
> > Hi Steffen,
> > 
> > Can you please try the following on top of 2/16?
> 
> How does this fit into the patch we're replying to?

Sorry but I don't quite get you.

If you look at a mixed source assembly listing for fc_host_dispatch() [1] we load
the the offset of 24 from %r12 into %r1 and then the beginning of %r1 into %r2
and crash. So if we now check if  job->request_len is smaller than uint32_t we
know we can't have a messagecode in the request and bail out early, instead of
accessing a possible NULL pointer. At least that's my analysis, feel free to
correct me if I'm wrong.

[1] Listing:
static int fc_bsg_host_dispatch(struct Scsi_Host *shost, struct bsg_job *job)
{
        struct fc_internal *i = to_fc_internal(shost->transportt);
        struct fc_bsg_request *bsg_request = job->request;
    5d54:       e3 10 c0 18 00 04       lg      %r1,24(%r12)
        struct fc_bsg_reply *bsg_reply = job->reply;
    5d5a:       e3 b0 c0 20 00 04       lg      %r11,32(%r12)
        int cmdlen = sizeof(uint32_t);  /* start with length of msgcode */
        int ret;

        /* check if we really have all the request data needed */
        if (job->request_len < cmdlen) {
    5d60:       a7 c4 00 48             jle     5df0 <fc_bsg_dispatch+0x1f0>
                ret = -ENOMSG;
                goto fail_host_msg;
        }

        /* Validate the host command */
        switch (bsg_request->msgcode) {
    5d64:       58 20 10 00             l       %r2,0(%r1)
    5d68:       c2 2f 80 00 00 03       clfi    %r2,2147483651
    5d6e:       a7 84 00 1a             je      5da2 <fc_bsg_dispatch+0x1a2>
    5d72:       a7 24 00 0a             jh      5d86 <fc_bsg_dispatch+0x186>
    5d76:       c0 19 80 00 00 01       iilf    %r1,2147483649
    5d7c:       ec 21 00 2b a0 77       clrj    %r2,%r1,10,5dd2 <fc_bsg_dispatch+0x1d2>
    5d82:       a7 f4 00 3b             j       5df8 <fc_bsg_dispatch+0x1f8>
    5d86:       c0 59 80 00 00 04       iilf    %r5,2147483652
    5d8c:       ec 25 00 0b 80 76       crj     %r2,%r5,8,5da2 <fc_bsg_dispatch+0x1a2>
    5d92:       c0 59 80 00 00 ff       iilf    %r5,2147483903
    5d98:       ec 25 00 13 80 76       crj     %r2,%r5,8,5dbe <fc_bsg_dispatch+0x1be>
    5d9e:       a7 f4 00 2d             j       5df8 <fc_bsg_dispatch+0x1f8>


Thanks,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@...e.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850