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

always initialize cause before return as error #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmwviormv
Copy link

There is a bug on certain situations that cause remain uninitialized, fdm will show these lines on my OpenBSD:

remote: D(L[A\A]A^A_A[]L3$L;1
fdm(35648) in free(): bogus pointer (double free?) 0x501a4c67622

This patch is to initialize cause on my situation (I didn't find any other uninitialized cause)

@nicm
Copy link
Owner

nicm commented Dec 5, 2022

Thanks but this is not enough - fetch_poll does not free cause when io_polln returns EAGAIN.

If you set cause on EAGAIN then you need need to check all calls to io_polln or io_poll and make sure they free it.

@fmwviormv
Copy link
Author

So we have a complex situation.
There are 2 types of EAGAIN error:

  1. poll return value -1 which leads to io_poll to return -1 with initialized cause
  2. poll return value 0 which leads to io_poll to return -1 with uninitialized cause

It is a little complex

@fmwviormv
Copy link
Author

So we have a complex situation. There are 2 types of EAGAIN error:

  1. poll return value -1 which leads to io_poll to return -1 with initialized cause
  2. poll return value 0 which leads to io_poll to return -1 with uninitialized cause

It is a little complex

I think we need to separate these 2 situations, because the second one is not a real error.
In my situation io_wait waits for SOCKS5 to recv few bytes, but SOCKS5 server might close the connection instead. In that situation there is no actual EAGAIN, so we should never wait again.

@nicm
Copy link
Owner

nicm commented Dec 7, 2022

Either it should always initialize cause when returning EAGAIN and all callers should free it, or it should never initialize cause when returning EAGAIN and no callers should free it.

It doesn't really matter which but you will need to change io_polln to do the same thing whether poll actually returns EAGAIN or we pretend it does when poll returns 0 and there is no timeout, and then check all the callers.

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

Successfully merging this pull request may close these issues.

2 participants