linux-kernel - Re: [PATCH 04/22] block: Abstract out bvec iterator

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:	Fri, 9 Aug 2013 16:16:01 -0400
From:	Ed Cashin <ecashin@...aid.com>
To:	Kent Overstreet <kmo@...erainc.com>
CC:	"axboe@...nel.dk" <axboe@...nel.dk>,
	"neilb@...e.de" <neilb@...e.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"dm-devel@...hat.com" <dm-devel@...hat.com>,
	"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>
Subject: Re: [PATCH 04/22] block: Abstract out bvec iterator

On Aug 8, 2013, at 9:05 PM, Kent Overstreet wrote:
...
> It's in the for-jens branch now.

Just examining the patches, I like the way it cleans up the aoe code.  I had a question about a new BUG added by the for-jens branch the read-response handling path of the aoe driver.

It looks like if a misbehaving AoE target has a bad (too high compared to the request) sector count, but sends a packet large enough for that sector count to seem legit in ktiocomplete, then the patched bvcpy will BUG.  An example would be the case where 1024 bytes was requested but a (bad but possible) AoE read response comes back with 4096 bytes in a jumbo frame.  Here's an excerpt from ktiocomplete:

     n = ahout->scnt << 9;
     switch (ahout->cmdstat) {
     case ATA_CMD_PIO_READ:
     case ATA_CMD_PIO_READ_EXT:
          if (skb->len < n) {
               pr_err("%s e%ld.%d.  skb->len=%d need=%ld\n",
                    "aoe: runt data size in read from",
                    (long) d->aoemajor, d->aoeminor,
                      skb->len, n);
               clear_bit(BIO_UPTODATE, &buf->bio->bi_flags);
               break;
          }
          bvcpy(skb, f->buf->bio, f->iter, n);

... and earlier in linux-bcache/for-jens aoecmd.c there's bvcpy ...

static void
bvcpy(struct sk_buff *skb, struct bio *bio, struct bvec_iter iter, long cnt)
{
     int soff = 0;
     struct bio_vec bv;

     BUG_ON(cnt > iter.bi_size);

It seems like it would be better to treat that case as another indication of a problem with the target that gets logged when the AoE response is ignored, just as happens for "runt data size".  That way people working on or trying out experimental AoE targets don't panic the initiator system.

-- 
  Ed Cashin
  ecashin@...aid.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/