Re: [PATCH v4 19/20] fuzz: add virtio-net fuzz target

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2157 bytes --]

On Wed, Oct 30, 2019 at 02:50:03PM +0000, Oleinik, Alexander wrote:
> +static void virtio_net_fuzz_multi(QTestState *s,
> +        const unsigned char *Data, size_t Size)
> +{
> +    typedef struct vq_action {
> +        uint8_t queue;
> +        uint8_t length;
> +        uint8_t write;
> +        uint8_t next;
> +        bool kick;
> +    } vq_action;
> +
> +    uint64_t req_addr[10];
> +    int reqi = 0;
> +    uint32_t free_head = 0;
> +
> +    QGuestAllocator *t_alloc = fuzz_qos_alloc;
> +
> +    QVirtioNet *net_if = fuzz_qos_obj;
> +    QVirtioDevice *dev = net_if->vdev;
> +    QVirtQueue *q;
> +    vq_action vqa;
> +    int iters = 0;
> +    while (true) {
> +        if (Size < sizeof(vqa)) {
> +            break;
> +        }

More concisely:

  while (Size < sizeof(vqa)) {

> +        memcpy(&vqa, Data, sizeof(vqa));
> +        vqa = *((vq_action *)Data);

The assignment is redundant after the memcpy.

> +        Data += sizeof(vqa);
> +        Size -= sizeof(vqa);
> +
> +        q = net_if->queues[vqa.queue % 3];
> +
> +        vqa.length = vqa.length >= Size ? Size :  vqa.length;
> +
> +        req_addr[reqi] = guest_alloc(t_alloc, vqa.length);
> +        qtest_memwrite(s, req_addr[reqi], Data, vqa.length);
> +        if (iters == 0) {

What is the difference between iters and reqi?  The values of these
variables are always identical so I think only one of them is needed.

> +            free_head = qvirtqueue_add(s, q, req_addr[reqi], vqa.length,
> +                    vqa.write, vqa.next);
> +        } else {
> +            qvirtqueue_add(s, q,
> +                    req_addr[reqi], vqa.length, vqa.write , vqa.next);
> +        }
> +        iters++;
> +        reqi++;
> +        if (iters == 10) {
> +            break;
> +        }
> +        Data += vqa.length;
> +        Size -= vqa.length;
> +    }
> +    if (iters) {
> +        qvirtqueue_kick(s, dev, q, free_head);
> +        qtest_clock_step_next(s);

Here we could run the main loop until free_head appears in the used
ring.  That should allow the request processing code path to be explored
more fully, but this it's okay to leave it like this for now.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]