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

[Ed Cashin <ecashin@coraid.com>]

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@coraid.com