linux-kernel - Re: [PATCH 2/3] virtio

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, 27 Oct 2015 22:09:07 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Christian Borntraeger <borntraeger@...ibm.com>
Cc:	Andy Lutomirski <luto@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Joerg Roedel <jroedel@...e.de>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	Sebastian Ott <sebott@...ux.vnet.ibm.com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Christoph Hellwig <hch@....de>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	KVM <kvm@...r.kernel.org>, David Woodhouse <dwmw2@...radead.org>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	linux-s390 <linux-s390@...r.kernel.org>
Subject: Re: [PATCH 2/3] virtio_ring: Support DMA APIs

On Tue, Oct 27, 2015 at 7:27 PM, Christian Borntraeger
<borntraeger@...ibm.com> wrote:
> Am 28.10.2015 um 10:17 schrieb Andy Lutomirski:
> @@ -423,27 +522,42 @@ EXPORT_SYMBOL_GPL(virtqueue_kick);
>>
>>  static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
>>  {
>> -     unsigned int i;
>> +     unsigned int i, j;
>> +     u16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
>>
>>       /* Clear data ptr. */
>> -     vq->data[head] = NULL;
>> +     vq->desc_state[head].data = NULL;
>>
>> -     /* Put back on free list: find end */
>> +     /* Put back on free list: unmap first-level descriptors and find end */
>>       i = head;
>>
>> -     /* Free the indirect table */
>> -     if (vq->vring.desc[i].flags & cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT))
>> -             kfree(phys_to_virt(virtio64_to_cpu(vq->vq.vdev, vq->vring.desc[i].addr)));
>> -
>> -     while (vq->vring.desc[i].flags & cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT)) {
>> +     while (vq->vring.desc[i].flags & nextflag) {
>> +             vring_unmap_one(vq, &vq->vring.desc[i]);
>>               i = virtio16_to_cpu(vq->vq.vdev, vq->vring.desc[i].next);
>>               vq->vq.num_free++;
>>       }
>>
>> +     vring_unmap_one(vq, &vq->vring.desc[i]);
>>       vq->vring.desc[i].next = cpu_to_virtio16(vq->vq.vdev, vq->free_head);
>>       vq->free_head = head;
>> +
>>       /* Plus final descriptor */
>>       vq->vq.num_free++;
>> +
>> +     /* Free the indirect table, if any, now that it's unmapped. */
>> +     if (vq->desc_state[head].indir_desc) {
>> +             struct vring_desc *indir_desc = vq->desc_state[head].indir_desc;
>> +             u32 len = vq->vring.desc[head].len;
>> +
>> +             BUG_ON(!(vq->vring.desc[head].flags & VRING_DESC_F_INDIRECT));
>> +             BUG_ON(len == 0 || len % sizeof(struct vring_desc));
>> +
>> +             for (j = 0; j < len / sizeof(struct vring_desc); j++)
>> +                     vring_unmap_one(vq, &indir_desc[j]);
>> +
>> +             kfree(vq->desc_state[head].indir_desc);
>> +             vq->desc_state[head].indir_desc = NULL;
>> +     }
>>  }
>
> something seems to be broken with indirect descriptors with that change

You're big endian, right?  I had an endian handling bug, and I'll fix it in v2.

--Andy
--
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/