Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalidate the ring region but use non initialize ring index #338

Closed
joshualin-petaio opened this issue Jan 27, 2022 · 3 comments · Fixed by #339
Closed

Invalidate the ring region but use non initialize ring index #338

joshualin-petaio opened this issue Jan 27, 2022 · 3 comments · Fixed by #339

Comments

@joshualin-petaio
Copy link
Contributor

Hi,

There have some question about use non initialized variable.

  1. At the virtqueue_get_available_buffer(). In order to get latest avail index. So invalidate the head ring region. But the head index is getting after invalidate the head ring region. It may cause that wrong region will be invalidate.

  2. The same issue is also happen in virtqueue_get_desc_size().

BR,
Joshua

@arnopo
Copy link
Collaborator

arnopo commented Jan 27, 2022

I suppose that you are spekking about this lines which should be inverted?

	/* Avail.ring is updated by master, invalidate it */
	VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx]);

	head_idx = vq->vq_available_idx++ & (vq->vq_nentries - 1);

Yes that seem to me something no safe if we are at the limit of 2 cache blocks.
@carlocaione : Do you confirm the potential issue?

@carlocaione
Copy link
Collaborator

Yes, this must be fixed.

The problem is that when we are using VRING_INVALIDATE the index head_idx is always 0, so indeed we are invalidating the wrong ring.

Thank you for spotting this. Let me know if you are going to open a PR or whether I should do it.

@joshualin-petaio
Copy link
Contributor Author

joshualin-petaio commented Jan 28, 2022

Hi Carlocaione,

I can send a PR.
And maybe you can check later.
This PR had include in #337 could you check it ?

Thanks,
Joshua

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants