Doug / smarc-fsl-linux-kernel

Download as
Browse Code ยป

Commit 6878c32e5cc0e40980abe51d1f02fb453e27493e

Authored by Konrad Rzeszutek Wilk
1 parent 8c9ce606a6
Exists in smarc-l5.0.0_1.0.0-ga and in 5 other branches imx_3.10.17_1.0.1_ga, imx_3.10.53_1.1.0_ga, imx_3.14.28_1.0.0_ga, smarc-imx_3.10.53_1.1.0_ga, smarc-imx_3.14.28_1.0.0_ga

xen/blkfront: Add WARN to deal with misbehaving backends. 

Part of the ring structure is the 'id' field which is under
control of the frontend. The frontend stamps it with "some"
value (this some in this implementation being a value less
than BLK_RING_SIZE), and when it gets a response expects
said value to be in the response structure. We have a check
for the id field when spolling new requests but not when
de-spolling responses.

We also add an extra check in add_id_to_freelist to make
sure that the 'struct request' was not NULL - as we cannot
pass a NULL to __blk_end_request_all, otherwise that crashes
(and all the operations that the response is dealing with
end up with __blk_end_request_all).

Lastly we also print the name of the operation that failed.

[v1: s/BUG/WARN/ suggested by Stefano]
[v2: Add extra check in add_id_to_freelist]
[v3: Redid op_name per Jan's suggestion]
[v4: add const * and add WARN on failure returns]
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Showing 1 changed file with 46 additions and 12 deletions Side-by-side Diff

drivers/block/xen-blkfront.c
Diff comments   View file @ 6878c32
... ... @@ -141,14 +141,36 @@
141 141 return free;
142 142 }
143 143  
144   -static void add_id_to_freelist(struct blkfront_info *info,
  144 +static int add_id_to_freelist(struct blkfront_info *info,
145 145 unsigned long id)
146 146 {
  147 + if (info->shadow[id].req.u.rw.id != id)
  148 + return -EINVAL;
  149 + if (info->shadow[id].request == NULL)
  150 + return -EINVAL;
147 151 info->shadow[id].req.u.rw.id = info->shadow_free;
148 152 info->shadow[id].request = NULL;
149 153 info->shadow_free = id;
  154 + return 0;
150 155 }
151 156  
  157 +static const char *op_name(int op)
  158 +{
  159 + static const char *const names[] = {
  160 + [BLKIF_OP_READ] = "read",
  161 + [BLKIF_OP_WRITE] = "write",
  162 + [BLKIF_OP_WRITE_BARRIER] = "barrier",
  163 + [BLKIF_OP_FLUSH_DISKCACHE] = "flush",
  164 + [BLKIF_OP_DISCARD] = "discard" };
  165 +
  166 + if (op < 0 || op >= ARRAY_SIZE(names))
  167 + return "unknown";
  168 +
  169 + if (!names[op])
  170 + return "reserved";
  171 +
  172 + return names[op];
  173 +}
152 174 static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
153 175 {
154 176 unsigned int end = minor + nr;
155 177  
156 178  
... ... @@ -746,20 +768,36 @@
746 768  
747 769 bret = RING_GET_RESPONSE(&info->ring, i);
748 770 id = bret->id;
  771 + /*
  772 + * The backend has messed up and given us an id that we would
  773 + * never have given to it (we stamp it up to BLK_RING_SIZE -
  774 + * look in get_id_from_freelist.
  775 + */
  776 + if (id >= BLK_RING_SIZE) {
  777 + WARN(1, "%s: response to %s has incorrect id (%ld)\n",
  778 + info->gd->disk_name, op_name(bret->operation), id);
  779 + /* We can't safely get the 'struct request' as
  780 + * the id is busted. */
  781 + continue;
  782 + }
749 783 req = info->shadow[id].request;
750 784  
751 785 if (bret->operation != BLKIF_OP_DISCARD)
752 786 blkif_completion(&info->shadow[id]);
753 787  
754   - add_id_to_freelist(info, id);
  788 + if (add_id_to_freelist(info, id)) {
  789 + WARN(1, "%s: response to %s (id %ld) couldn't be recycled!\n",
  790 + info->gd->disk_name, op_name(bret->operation), id);
  791 + continue;
  792 + }
755 793  
756 794 error = (bret->status == BLKIF_RSP_OKAY) ? 0 : -EIO;
757 795 switch (bret->operation) {
758 796 case BLKIF_OP_DISCARD:
759 797 if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
760 798 struct request_queue *rq = info->rq;
761   - printk(KERN_WARNING "blkfront: %s: discard op failed\n",
762   - info->gd->disk_name);
  799 + printk(KERN_WARNING "blkfront: %s: %s op failed\n",
  800 + info->gd->disk_name, op_name(bret->operation));
763 801 error = -EOPNOTSUPP;
764 802 info->feature_discard = 0;
765 803 info->feature_secdiscard = 0;
766 804  
... ... @@ -771,18 +809,14 @@
771 809 case BLKIF_OP_FLUSH_DISKCACHE:
772 810 case BLKIF_OP_WRITE_BARRIER:
773 811 if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
774   - printk(KERN_WARNING "blkfront: %s: write %s op failed\n",
775   - info->flush_op == BLKIF_OP_WRITE_BARRIER ?
776   - "barrier" : "flush disk cache",
777   - info->gd->disk_name);
  812 + printk(KERN_WARNING "blkfront: %s: %s op failed\n",
  813 + info->gd->disk_name, op_name(bret->operation));
778 814 error = -EOPNOTSUPP;
779 815 }
780 816 if (unlikely(bret->status == BLKIF_RSP_ERROR &&
781 817 info->shadow[id].req.u.rw.nr_segments == 0)) {
782   - printk(KERN_WARNING "blkfront: %s: empty write %s op failed\n",
783   - info->flush_op == BLKIF_OP_WRITE_BARRIER ?
784   - "barrier" : "flush disk cache",
785   - info->gd->disk_name);
  818 + printk(KERN_WARNING "blkfront: %s: empty %s op failed\n",
  819 + info->gd->disk_name, op_name(bret->operation));
786 820 error = -EOPNOTSUPP;
787 821 }
788 822 if (unlikely(error)) {