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

[Stefan Hajnoczi]

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.

signature.asc
Description: PGP signature