nicm / fdm

fdm source code
270 stars 50 forks source link

always initialize cause before return as error #119

Open fmwviormv opened 1 year ago

fmwviormv commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.