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

Access violation in interactive service #232

Closed
lstipakov opened this issue Jan 31, 2023 · 3 comments
Closed

Access violation in interactive service #232

lstipakov opened this issue Jan 31, 2023 · 3 comments

Comments

@lstipakov
Copy link
Member

When iservice is unable to apply "block-outside-dns", access violation happens during undo phase.

Sieppaa

Steps to reproduce:

There is a compatibility issue with dco driver and Sonicwall driver, but interactive service should not crash regardless of it. The access violation seem to happen inside WFP, but that does not excuse us :)

@lstipakov
Copy link
Member Author

ping @selvanair

@selvanair
Copy link
Contributor

Does the value of engine handle at this point the same as the one we got from the open call? We do check for error while opening so it has to be a valid handle. If this shows up only when sonicwall/dco compat issue is present, it could be related, no? Like a broken interface that triggers a crash inside WFP when referred to by block-dns?

Will try installing SonicWall client later.

@selvanair
Copy link
Contributor

selvanair commented Jan 31, 2023

I cannot reproduce this as dco adapter works for me and block-dns succeeds with sonicwall client installed. Tried re-installing 2.6.0 as a clean install, deleted dco adapter and recreated both with sonic wall installed. No misbehaviour.

@lev I was tetsing with SonicWall's NetExtender not GlobalVPN... yikes.. Reproduced the crash with the latter installed (and its offensive DNE filter).

Anyway, on re-reading your comment, I realized the initial failure is while adding the filter. In that case we do not add the engine to the undo list, and this undo call during termination of openvpn should not happen at all.

Edit:
It turns out that add_block_dns_filters() succeeds (or at least returns success) in spite of the broken adapter state, so we do save the engine handle in the undo list. But before returning from the service, we try to change the metric of the adapter and fail with "no such element". We dutifully clean up block-dns filters but forget to clear the undo list. When we get a call to undo following openvpn.exe's exit, the undo list has an engine handle that's already freed which causes an access violation in FwpmEngineClose0().

selvanair added a commit to selvanair/openvpn that referenced this issue Feb 1, 2023
- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN#232

Signed-off-by: Selva Nair <selva.nair@gmail.com>
selvanair added a commit to selvanair/openvpn that referenced this issue Feb 1, 2023
- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN#232

Signed-off-by: Selva Nair <selva.nair@gmail.com>
lstipakov pushed a commit to lstipakov/openvpn that referenced this issue Feb 1, 2023
- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN#232

Signed-off-by: Selva Nair <selva.nair@gmail.com>
lstipakov pushed a commit to lstipakov/openvpn that referenced this issue Feb 2, 2023
- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN#232

v2: Split add and delete functions and reuse the delete
function for cleanup.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
cron2 pushed a commit to cron2/openvpn that referenced this issue Feb 2, 2023
- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN/openvpn#232

v2: Split add and delete functions and reuse the delete
function for cleanup.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230201170735.2266851-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26130.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b761cb9)
@cron2 cron2 closed this as completed in b761cb9 Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants